Uh oh!
There was an error while loading. Please reload this page.
src: refactor EncodeValidUtf8 to reuse non-ASCII UTF-8 path - #63587
src: refactor EncodeValidUtf8 to reuse non-ASCII UTF-8 path#63587araujogui wants to merge 4 commits into
Conversation
92897bc to
a2ccfb5CompareThere was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR optimizes UTF-8 decoding in the encoding binding by avoiding redundant UTF-8 validation when input has already been validated.
Changes:
- Added
StringBytes::EncodeValidatedUTF8()API for encoding already-validated UTF-8. - Refactored the fast UTF-8→UTF-16 conversion into a shared helper (
EncodeKnownValidNonAsciiUTF8). - Updated
BindingData::DecodeUTF8()to use the new API after performing UTF-8 validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/string_bytes.h | Declares a new EncodeValidatedUTF8() entry point for already-validated UTF-8. |
| src/string_bytes.cc | Implements EncodeValidatedUTF8() and factors out the fast conversion helper used by both code paths. |
| src/encoding_binding.cc | Uses EncodeValidatedUTF8() after explicit UTF-8 validation to avoid re-validation overhead. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #63587 +/- ##
=======================================
Coverage 90.34% 90.35% =======================================
Files 732 732 Lines 236689 236681 -8 Branches 44586 44584 -2 =======================================
+ Hits 213839 213842 +3 + Misses 14550 14548 -2 + Partials 8300 8291 -9
🚀 New features to boost your workflow:
|
727b683 to
6a919f6Comparearaujogui
commented
May 27, 2026
CC @nodejs/performance |
6a919f6 to
183258aCompareI also tried to fix this in #63231, but this pr more generally, good work! |
araujogui
commented
May 27, 2026
Uh, my bad, I haven't seen it. I'm okay with closing this PR in favour of yours. |
No problem at all, thanks for your effort. This PR seems like a better solution to me, @addaleax, Do you have any thoughts? |
b711577 to
e41315dCompare@mertcanaltin@addaleax I rebased the branch, it's just a small DRY refactoring now |
aabbe55 to
c02c51dCompareSigned-off-by: Guilherme Araújo <arauujogui@gmail.com>
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
c02c51d to
f361b77Compare
Upstream PR #63231 added
StringBytes::EncodeValidUtf8but duplicated theEncodeTwoByteStringblock already present inStringBytes::Encode.This PR refactors that by extracting a shared
EncodeValidNonAsciiUtf8helper, called from bothEncodeValidUtf8and the existingEncodeUTF-8 path.