diff options
author | Anna Henningsen <anna@addaleax.net> | 2017-08-12 16:49:31 +0200 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2017-08-17 22:55:01 +0200 |
commit | 4766fcb96d9459b7db8fb201d6ac44d6957b4df0 (patch) | |
tree | 85d33d636ce3c8adf3528afcb28e5c4d03f5a7a7 | |
parent | 0d9afa0288ccff50edf66a210f2e77d710a0ddca (diff) | |
download | android-node-v8-4766fcb96d9459b7db8fb201d6ac44d6957b4df0.tar.gz android-node-v8-4766fcb96d9459b7db8fb201d6ac44d6957b4df0.tar.bz2 android-node-v8-4766fcb96d9459b7db8fb201d6ac44d6957b4df0.zip |
http2: minor refactor of passing headers to JS
- Make `ExternalString::New` return a `MaybeLocal`. Failing is
left up to the caller.
- Allow creating internalized strings for short header names
to reduce memory consumption and increase performance.
- Use persistent storage for statically allocated header names.
PR-URL: https://github.com/nodejs/node/pull/14808
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
-rw-r--r-- | benchmark/http2/headers.js | 13 | ||||
-rw-r--r-- | src/env.h | 5 | ||||
-rwxr-xr-x | src/node_http2.cc | 6 | ||||
-rwxr-xr-x | src/node_http2.h | 44 |
4 files changed, 54 insertions, 14 deletions
diff --git a/benchmark/http2/headers.js b/benchmark/http2/headers.js index 09449d1e92..078e7a356a 100644 --- a/benchmark/http2/headers.js +++ b/benchmark/http2/headers.js @@ -5,7 +5,7 @@ const PORT = common.PORT; var bench = common.createBenchmark(main, { n: [1e3], - nheaders: [100, 1000], + nheaders: [0, 10, 100, 1000], }, { flags: ['--expose-http2', '--no-warnings'] }); function main(conf) { @@ -14,7 +14,16 @@ function main(conf) { const http2 = require('http2'); const server = http2.createServer(); - const headersObject = { ':path': '/' }; + const headersObject = { + ':path': '/', + ':scheme': 'http', + 'accept-encoding': 'gzip, deflate', + 'accept-language': 'en', + 'content-type': 'text/plain', + 'referer': 'https://example.org/', + 'user-agent': 'SuperBenchmarker 3000' + }; + for (var i = 0; i < nheaders; i++) { headersObject[`foo${i}`] = `some header value ${i}`; } @@ -39,6 +39,9 @@ #include <stdint.h> #include <vector> #include <stack> +#include <unordered_map> + +struct nghttp2_rcbuf; namespace node { @@ -341,6 +344,8 @@ class IsolateData { #undef VS #undef VP + std::unordered_map<nghttp2_rcbuf*, v8::Eternal<v8::String>> http2_static_strs; + private: #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) diff --git a/src/node_http2.cc b/src/node_http2.cc index 16c7270342..9308e9e68e 100755 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -901,8 +901,10 @@ void Http2Session::OnHeaders(Nghttp2Stream* stream, while (headers != nullptr && j < arraysize(argv) / 2) { nghttp2_header_list* item = headers; // The header name and value are passed as external one-byte strings - name_str = ExternalHeader::New(isolate, item->name); - value_str = ExternalHeader::New(isolate, item->value); + name_str = + ExternalHeader::New<true>(env(), item->name).ToLocalChecked(); + value_str = + ExternalHeader::New<false>(env(), item->value).ToLocalChecked(); argv[j * 2] = name_str; argv[j * 2 + 1] = value_str; headers = item->next; diff --git a/src/node_http2.h b/src/node_http2.h index 3ff2d33a7b..107e87a2dc 100755 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -472,24 +472,48 @@ class ExternalHeader : return vec_.len; } - static Local<String> New(Isolate* isolate, nghttp2_rcbuf* buf) { - EscapableHandleScope scope(isolate); + static inline + MaybeLocal<String> GetInternalizedString(Environment* env, + const nghttp2_vec& vec) { + return String::NewFromOneByte(env->isolate(), + vec.base, + v8::NewStringType::kInternalized, + vec.len); + } + + template<bool may_internalize> + static MaybeLocal<String> New(Environment* env, nghttp2_rcbuf* buf) { + if (nghttp2_rcbuf_is_static(buf)) { + auto& static_str_map = env->isolate_data()->http2_static_strs; + v8::Eternal<v8::String>& eternal = static_str_map[buf]; + if (eternal.IsEmpty()) { + Local<String> str = + GetInternalizedString(env, nghttp2_rcbuf_get_buf(buf)) + .ToLocalChecked(); + eternal.Set(env->isolate(), str); + return str; + } + return eternal.Get(env->isolate()); + } + nghttp2_vec vec = nghttp2_rcbuf_get_buf(buf); if (vec.len == 0) { nghttp2_rcbuf_decref(buf); - return scope.Escape(String::Empty(isolate)); + return String::Empty(env->isolate()); } - ExternalHeader* h_str = new ExternalHeader(buf); - MaybeLocal<String> str = String::NewExternalOneByte(isolate, h_str); - isolate->AdjustAmountOfExternalAllocatedMemory(vec.len); + if (may_internalize && vec.len < 64) { + // This is a short header name, so there is a good chance V8 already has + // it internalized. + return GetInternalizedString(env, vec); + } - if (str.IsEmpty()) { + ExternalHeader* h_str = new ExternalHeader(buf); + MaybeLocal<String> str = String::NewExternalOneByte(env->isolate(), h_str); + if (str.IsEmpty()) delete h_str; - return scope.Escape(String::Empty(isolate)); - } - return scope.Escape(str.ToLocalChecked()); + return str; } private: |