Skip to content

Commit 4c0195e

Browse files
bnoordhuisbnoordhuis
committed
http: remove MethodToString()
The list of supported HTTP methods is available in JS land now so there is no longer any need to pass a stringified version of the method to the parser callback, it can look up the method name for itself. Saves a call to v8::Eternal::Get() in the common case and a costly v8::String::NewFromOneByte() in the uncommon case.
1 parent 6100228 commit 4c0195e

4 files changed

Lines changed: 16 additions & 45 deletions

File tree

‎lib/_http_common.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ function parserOnHeadersComplete(info) {
9292

9393
if (info.method) {
9494
// server only
95-
parser.incoming.method = info.method;
95+
parser.incoming.method = HTTPParser.methods[info.method];
9696
} else {
9797
// client only
9898
parser.incoming.statusCode = info.statusCode;

‎src/env.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,6 @@ namespace node {
5151
// Strings are per-isolate primitives but Environment proxies them
5252
// for the sake of convenience.
5353
#define PER_ISOLATE_STRING_PROPERTIES(V) \
54-
V(DELETE_string, "DELETE") \
55-
V(GET_string, "GET") \
56-
V(HEAD_string, "HEAD") \
57-
V(POST_string, "POST") \
58-
V(PUT_string, "PUT") \
5954
V(address_string, "address") \
6055
V(atime_string, "atime") \
6156
V(birthtime_string, "birthtime") \

‎src/node_http_parser.cc‎

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ using v8::Integer;
6464
using v8::Local;
6565
using v8::Object;
6666
using v8::String;
67+
using v8::Uint32;
6768
using v8::Value;
6869

6970
const uint32_t kOnHeaders = 0;
@@ -88,33 +89,6 @@ const uint32_t kOnMessageComplete = 3;
8889
int name##_(const char* at, size_t length)
8990

9091

91-
// Call this function only when there is a valid HandleScope on the stack
92-
// somewhere.
93-
inline Local<String> MethodToString(Environment* env, uint32_t method) {
94-
// XXX(bnoordhuis) Predicated on the observation that 99.9% of all HTTP
95-
// requests are either GET, HEAD or POST. I threw in DELETE and PUT for
96-
// good measure.
97-
switch (method) {
98-
case HTTP_DELETE: return env->DELETE_string();
99-
case HTTP_GET: return env->GET_string();
100-
case HTTP_HEAD: return env->HEAD_string();
101-
case HTTP_POST: return env->POST_string();
102-
case HTTP_PUT: return env->PUT_string();
103-
}
104-
105-
switch (method) {
106-
#define V(num, name, string) \
107-
case HTTP_ ## name: \
108-
return FIXED_ONE_BYTE_STRING(node_isolate, #string);
109-
HTTP_METHOD_MAP(V)
110-
#undef V
111-
}
112-
113-
// Unreachable, http_parser parses only a restricted set of request methods.
114-
return FIXED_ONE_BYTE_STRING(node_isolate, "UNKNOWN_METHOD");
115-
}
116-
117-
11892
// helper class for the Parser
11993
struct StringPtr {
12094
StringPtr() {
@@ -276,7 +250,7 @@ class Parser : public WeakObject {
276250
// METHOD
277251
if (parser_.type == HTTP_REQUEST) {
278252
message_info->Set(env()->method_string(),
279-
MethodToString(env(), parser_.method));
253+
Uint32::NewFromUnsigned(parser_.method));
280254
}
281255

282256
// STATUS

‎test/simple/test-http-parser.js‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ var CRLF = '\r\n';
2828
var REQUEST = HTTPParser.REQUEST;
2929
var RESPONSE = HTTPParser.RESPONSE;
3030

31+
var methods = HTTPParser.methods;
32+
3133
var kOnHeaders = HTTPParser.kOnHeaders | 0;
3234
var kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
3335
var kOnBody = HTTPParser.kOnBody | 0;
@@ -98,7 +100,7 @@ function expectBody(expected) {
98100
var parser = newParser(REQUEST);
99101

100102
parser[kOnHeadersComplete] = mustCall(function(info) {
101-
assert.equal(info.method, 'GET');
103+
assert.equal(info.method, methods.indexOf('GET'));
102104
assert.equal(info.url || parser.url, '/hello');
103105
assert.equal(info.versionMajor, 1);
104106
assert.equal(info.versionMinor, 1);
@@ -200,7 +202,7 @@ function expectBody(expected) {
200202
var parser = newParser(REQUEST);
201203

202204
parser[kOnHeadersComplete] = mustCall(function(info) {
203-
assert.equal(info.method, 'POST');
205+
assert.equal(info.method, methods.indexOf('POST'));
204206
assert.equal(info.url || parser.url, '/it');
205207
assert.equal(info.versionMajor, 1);
206208
assert.equal(info.versionMinor, 1);
@@ -232,7 +234,7 @@ function expectBody(expected) {
232234
var parser = newParser(REQUEST);
233235

234236
parser[kOnHeadersComplete] = mustCall(function(info) {
235-
assert.equal(info.method, 'GET');
237+
assert.equal(info.method, methods.indexOf('GET'));
236238
assert.equal(info.versionMajor, 1);
237239
assert.equal(info.versionMinor, 0);
238240
assert.deepEqual(info.headers || parser.headers,
@@ -261,7 +263,7 @@ function expectBody(expected) {
261263
var parser = newParser(REQUEST);
262264

263265
parser[kOnHeadersComplete] = mustCall(function(info) {
264-
assert.equal(info.method, 'GET');
266+
assert.equal(info.method, methods.indexOf('GET'));
265267
assert.equal(info.url || parser.url, '/foo/bar/baz?quux=42#1337');
266268
assert.equal(info.versionMajor, 1);
267269
assert.equal(info.versionMinor, 0);
@@ -293,7 +295,7 @@ function expectBody(expected) {
293295
var parser = newParser(REQUEST);
294296

295297
parser[kOnHeadersComplete] = mustCall(function(info) {
296-
assert.equal(info.method, 'POST');
298+
assert.equal(info.method, methods.indexOf('POST'));
297299
assert.equal(info.url || parser.url, '/it');
298300
assert.equal(info.versionMajor, 1);
299301
assert.equal(info.versionMinor, 1);
@@ -328,7 +330,7 @@ function expectBody(expected) {
328330
var parser = newParser(REQUEST);
329331

330332
parser[kOnHeadersComplete] = mustCall(function(info) {
331-
assert.equal(info.method, 'POST');
333+
assert.equal(info.method, methods.indexOf('POST'));
332334
assert.equal(info.url || parser.url, '/it');
333335
assert.equal(info.versionMajor, 1);
334336
assert.equal(info.versionMinor, 1);
@@ -364,7 +366,7 @@ function expectBody(expected) {
364366
var parser = newParser(REQUEST);
365367

366368
parser[kOnHeadersComplete] = mustCall(function(info) {
367-
assert.equal(info.method, 'POST');
369+
assert.equal(info.method, methods.indexOf('POST'));
368370
assert.equal(info.url || parser.url, '/it');
369371
assert.equal(info.versionMajor, 1);
370372
assert.equal(info.versionMinor, 1);
@@ -421,7 +423,7 @@ function expectBody(expected) {
421423
var parser = newParser(REQUEST);
422424

423425
parser[kOnHeadersComplete] = mustCall(function(info) {
424-
assert.equal(info.method, 'POST');
426+
assert.equal(info.method, methods.indexOf('POST'));
425427
assert.equal(info.url || parser.url, '/helpme');
426428
assert.equal(info.versionMajor, 1);
427429
assert.equal(info.versionMinor, 1);
@@ -477,7 +479,7 @@ function expectBody(expected) {
477479
var parser = newParser(REQUEST);
478480

479481
parser[kOnHeadersComplete] = mustCall(function(info) {
480-
assert.equal(info.method, 'POST');
482+
assert.equal(info.method, methods.indexOf('POST'));
481483
assert.equal(info.url || parser.url, '/it');
482484
assert.equal(info.versionMajor, 1);
483485
assert.equal(info.versionMinor, 1);
@@ -523,7 +525,7 @@ function expectBody(expected) {
523525
'pong');
524526

525527
function onHeadersComplete1(info) {
526-
assert.equal(info.method, 'PUT');
528+
assert.equal(info.method, methods.indexOf('PUT'));
527529
assert.equal(info.url, '/this');
528530
assert.equal(info.versionMajor, 1);
529531
assert.equal(info.versionMinor, 1);
@@ -533,7 +535,7 @@ function expectBody(expected) {
533535
};
534536

535537
function onHeadersComplete2(info) {
536-
assert.equal(info.method, 'POST');
538+
assert.equal(info.method, methods.indexOf('POST'));
537539
assert.equal(info.url, '/that');
538540
assert.equal(info.versionMajor, 1);
539541
assert.equal(info.versionMinor, 0);

0 commit comments

Comments
 (0)