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.

diff --git a/lib/_http_common.js b/lib/_http_common.js
index 3bc1346..2fb5bd8 100644 (file)
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -92,7 +92,7 @@ function parserOnHeadersComplete(info) {
 
   if (info.method) {
     // server only
-    parser.incoming.method = info.method;
+    parser.incoming.method = HTTPParser.methods[info.method];
   } else {
     // client only
     parser.incoming.statusCode = info.statusCode;

diff --git a/src/env.h b/src/env.h
index 415e3dd..5814b44 100644 (file)
--- a/src/env.h
+++ b/src/env.h
@@ -51,11 +51,6 @@ namespace node {
 // Strings are per-isolate primitives but Environment proxies them
 // for the sake of convenience.
 #define PER_ISOLATE_STRING_PROPERTIES(V)                                      \
-  V(DELETE_string, "DELETE")                                                  \
-  V(GET_string, "GET")                                                        \
-  V(HEAD_string, "HEAD")                                                      \
-  V(POST_string, "POST")                                                      \
-  V(PUT_string, "PUT")                                                        \
   V(address_string, "address")                                                \
   V(atime_string, "atime")                                                    \
   V(birthtime_string, "birthtime")                                            \

diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 5dbd1da..d6b0090 100644 (file)
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -64,6 +64,7 @@ using v8::Integer;
 using v8::Local;
 using v8::Object;
 using v8::String;
+using v8::Uint32;
 using v8::Value;
 
 const uint32_t kOnHeaders = 0;
@@ -88,33 +89,6 @@ const uint32_t kOnMessageComplete = 3;
   int name##_(const char* at, size_t length)
 
 
-// Call this function only when there is a valid HandleScope on the stack
-// somewhere.
-inline Local<String> MethodToString(Environment* env, uint32_t method) {
-  // XXX(bnoordhuis) Predicated on the observation that 99.9% of all HTTP
-  // requests are either GET, HEAD or POST. I threw in DELETE and PUT for
-  // good measure.
-  switch (method) {
-    case HTTP_DELETE: return env->DELETE_string();
-    case HTTP_GET: return env->GET_string();
-    case HTTP_HEAD: return env->HEAD_string();
-    case HTTP_POST: return env->POST_string();
-    case HTTP_PUT: return env->PUT_string();
-  }
-
-  switch (method) {
-#define V(num, name, string)                                                  \
-    case HTTP_ ## name:                                                       \
-      return FIXED_ONE_BYTE_STRING(node_isolate, #string);
-    HTTP_METHOD_MAP(V)
-#undef V
-  }
-
-  // Unreachable, http_parser parses only a restricted set of request methods.
-  return FIXED_ONE_BYTE_STRING(node_isolate, "UNKNOWN_METHOD");
-}
-
-
 // helper class for the Parser
 struct StringPtr {
   StringPtr() {
@@ -276,7 +250,7 @@ class Parser : public WeakObject {
     // METHOD
     if (parser_.type == HTTP_REQUEST) {
       message_info->Set(env()->method_string(),
-                        MethodToString(env(), parser_.method));
+                        Uint32::NewFromUnsigned(parser_.method));
     }
 
     // STATUS

diff --git a/test/simple/test-http-parser.js b/test/simple/test-http-parser.js
index e0f1fbf..7536f77 100644 (file)
--- a/test/simple/test-http-parser.js
+++ b/test/simple/test-http-parser.js
@@ -28,6 +28,8 @@ var CRLF = '\r\n';
 var REQUEST = HTTPParser.REQUEST;
 var RESPONSE = HTTPParser.RESPONSE;
 
+var methods = HTTPParser.methods;
+
 var kOnHeaders = HTTPParser.kOnHeaders | 0;
 var kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
 var kOnBody = HTTPParser.kOnBody | 0;
@@ -98,7 +100,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'GET');
+    assert.equal(info.method, methods.indexOf('GET'));
     assert.equal(info.url || parser.url, '/hello');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 1);
@@ -200,7 +202,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'POST');
+    assert.equal(info.method, methods.indexOf('POST'));
     assert.equal(info.url || parser.url, '/it');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 1);
@@ -232,7 +234,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'GET');
+    assert.equal(info.method, methods.indexOf('GET'));
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 0);
     assert.deepEqual(info.headers || parser.headers,
@@ -261,7 +263,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'GET');
+    assert.equal(info.method, methods.indexOf('GET'));
     assert.equal(info.url || parser.url, '/foo/bar/baz?quux=42#1337');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 0);
@@ -293,7 +295,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'POST');
+    assert.equal(info.method, methods.indexOf('POST'));
     assert.equal(info.url || parser.url, '/it');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 1);
@@ -328,7 +330,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'POST');
+    assert.equal(info.method, methods.indexOf('POST'));
     assert.equal(info.url || parser.url, '/it');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 1);
@@ -364,7 +366,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'POST');
+    assert.equal(info.method, methods.indexOf('POST'));
     assert.equal(info.url || parser.url, '/it');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 1);
@@ -421,7 +423,7 @@ function expectBody(expected) {
     var parser = newParser(REQUEST);
 
     parser[kOnHeadersComplete] = mustCall(function(info) {
-      assert.equal(info.method, 'POST');
+      assert.equal(info.method, methods.indexOf('POST'));
       assert.equal(info.url || parser.url, '/helpme');
       assert.equal(info.versionMajor, 1);
       assert.equal(info.versionMinor, 1);
@@ -477,7 +479,7 @@ function expectBody(expected) {
   var parser = newParser(REQUEST);
 
   parser[kOnHeadersComplete] = mustCall(function(info) {
-    assert.equal(info.method, 'POST');
+    assert.equal(info.method, methods.indexOf('POST'));
     assert.equal(info.url || parser.url, '/it');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 1);
@@ -523,7 +525,7 @@ function expectBody(expected) {
       'pong');
 
   function onHeadersComplete1(info) {
-    assert.equal(info.method, 'PUT');
+    assert.equal(info.method, methods.indexOf('PUT'));
     assert.equal(info.url, '/this');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 1);
@@ -533,7 +535,7 @@ function expectBody(expected) {
   };
 
   function onHeadersComplete2(info) {
-    assert.equal(info.method, 'POST');
+    assert.equal(info.method, methods.indexOf('POST'));
     assert.equal(info.url, '/that');
     assert.equal(info.versionMajor, 1);
     assert.equal(info.versionMinor, 0);