url: speed up URLPattern - #65364
Conversation
|
Review requested:
|
jasnell
left a comment
There was a problem hiding this comment.
AI agents are not permitted to use Signed-off-by
| length); | ||
| } | ||
| return String::NewFromUtf8( | ||
| isolate, view.data(), NewStringType::kNormal, length); |
There was a problem hiding this comment.
We already have ToV8Value that takes an std::string_view
There was a problem hiding this comment.
Switched these call sites to ToV8Value and dropped the local helper.
| // and destroy the view before creating any V8 heap objects. | ||
| void CopyV8StringToBuffer(Isolate* isolate, | ||
| Local<String> str, | ||
| MaybeStackBuffer<char>& buffer) { |
There was a problem hiding this comment.
Project convention is to pass pointers when mutating and to only pass refs as const ref
There was a problem hiding this comment.
CopyV8StringToBuffer now takes MaybeStackBuffer<char>*.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65364 +/- ##
==========================================
- Coverage 90.31% 90.13% -0.19%
==========================================
Files 751 752 +1
Lines 249956 251852 +1896
Branches 47204 47354 +150
==========================================
+ Hits 225745 227002 +1257
- Misses 15612 16175 +563
- Partials 8599 8675 +76
🚀 New features to boost your workflow:
|
351c14d to
52a2a71
Compare
|
Removed the |
52a2a71 to
a2e81de
Compare
Avoid extra UTF-8 copies on constructor, test, and exec; convert regexp inputs with ToV8Value; and assign URLPatternInit fields by interned key instead of string-comparing each component name. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com> Assisted-by: Cursor Co-authored-by: Cursor Agent <cursoragent@cursor.com>
b1e8b1d to
ed512d0
Compare
This speeds up WHATWG
URLPatternconstructor,test(), andexec()without changing observable behavior.Independent of the
new URL()parse PR (#65361) and theURLSearchParamsPR (#65363).What changed
C++ only (
src/node_url_pattern.cc):ValueView+memcpyinto a stack buffer and pass astring_viewinto Ada. Drop the extrastd::stringcopy thatBufferValue/Utf8Value::ToString()used to make. Non-ASCII still goes throughUtf8Value.ToV8Valueand useisolate->GetCurrentContext()instead ofEnvironment::GetCurrent.test/exec: share argument parsing so both paths keep the same WebIDL null/undefined/"null"baseURL behavior.Ada still owns its parsed state;
string_views only live for the duration of the C++ call.ValueViewis destroyed before any V8 heap allocation.Tests
test/parallel/test-urlpattern.js,test-urlpattern-types.js,test-urlpattern-invalidthis.js, andtest-urlpattern-fast-path.jsall passtest/wpt/test-urlpattern.js: 743 passed, 0 unexpected failuresReview follow-up
NewStringFromUtf8Viewhelper in favor ofToV8ValueCopyV8StringToBuffertakes a pointer, matching project convention for mutating argumentsSigned-off-byare Yagiz Nizipli (human DCO)Assisted-by: Cursor