[nigrosimone]

Follow-up of #1332, with the approach you suggested there.

NativeString wrote every string twice: Utf8Length() to size it, then WriteUtf8V2() into the pool. Now it calls WriteUtf8V2() once, straight into what is left of the pool, and advances the offset by the bytes written. If not all characters were processed the string did not fit, and it is measured and written again into malloc'd memory, like before. Before V8 14 it does the same with WriteUtf8() and nchars_ref.

tests/strings.js compares the bytes on the wire for ASCII, Latin-1, BMP, astral, NUL, empty and strings past the pool. It passes on master and on this branch, and fails the two past-the-pool cases on a build that skips the did-it-fit check. It runs in build.yml after smoke.js.

Measured on Node 26 win-x64, min of 7 alternating runs, two rounds in reversed order: req.getHeader with a 10 byte key from 66-69 to 59 ns, with a 1 KB key from 62-66 to 50-51 ns. It also compiles against the Node 22 and 24 headers.

[uNetworkingAB]

We can still add the StringView and ASCII test as a faster fast path but then we need to introduce baically NativeString's that exist in a group so either all succeed or all fail with the fallback. And those can only be used for String-only arguments (functions that only take Strings like getHeader, writeHeader)

[uNetworkingAB]

Wait, no they work on all functions but the strings will need to be parsed out last of all. In a group.

[ValoChet]

I don't know if you missed it, but #1302 was kind of doing what this PR does, while preventing the worst slow path.

[nigrosimone]

I don't know if you missed it, but #1302 was kind of doing what this PR does, while preventing the worst slow path.

You are right, I missed #1302, sorry. It is the same idea, and your worst-case check avoids a bad case that #1335 has: when the string is bigger than what is left of the pool, the first write fills the pool and is thrown away (up to 128 KB), then the string is measured and written again.
A check string->Length() > capacity before the first write would skip it, because UTF-8 is never shorter than the number of characters. Do you want to rebase #1302 on master with it? The ArrayBuffer::Data() part is not in master yet.

[ValoChet]

I'll let @uNetworkingAB decide what he wants to do.
We could argue that the worst case should never happen, because doing .end(128 KB) is bad anyway.
But the pool system rewrite and ArrayBuffer::Data() part may be wanted.

[uNetworkingAB]

Ah! That's a good one for slow slow path

[uNetworkingAB]

The drawback of 3x requirement is that you hit the fast path less likely. So if we have 128kb we only get a maximum of 40kb per string. So it becomes more sensitive to lowering the pool size (if you make the pool size 1kb you have almost no room). But it's still a good trade off, most of that pool will never be used and is not even mapped in at all. Ok, yeah let's do it.

[nigrosimone]

The Length() check does not have this drawback: the first write is still tried for every string that can fit, so ASCII keeps the fast path up to all the free pool. It skips only the strings that surely do not fit, with more characters than free bytes. What is left is a non-ASCII string a bit bigger than the free space, and there the waste is at most the free space.
@ValoChet do you want to add it to #1302 together with the ArrayBuffer::Data() part? Otherwise I can open a small PR.

[ValoChet]

@nigrosimone My PR has a different architecture from what has been merged.
I don't know if @uNetworkingAB wants to adopt #1302 or keep the current one and enhance it.
Either #1302 gets merged, or you are free to do your own PR based on it.

#1302 actually do:

• Fast path (only WriteUtf8(pool)) for Latin-1 <= 64 KB and UTF-16 <= 42 KB
• Mid path (Utf8Length() + WriteUtf8(pool)) for strings <= 128 KB
• Slow path (Utf8Length() + WriteUtf8(malloc)) for strings > 128 KB

[uNetworkingAB]

Fast path (only WriteUtf8(pool)) for Latin-1 <= 64 KB and UTF-16 <= 42 KB
Mid path (Utf8Length() + WriteUtf8(pool)) for strings <= 128 KB
Slow path (Utf8Length() + WriteUtf8(malloc)) for strings > 128 KB

That's ideal. But your PR is really messy and changes naming of things without any need (allocated gets renamed to needsFree without any motivation). If you want that PR to land, please make it a minimal change that only changes logic, not names.

[ValoChet]

allocated gets renamed to needsFree

I renamed it "needsFree" because its purpose is different from "allocated".
allocated was set to true on either pool or malloc allocation.
needsFree is set to true only on malloc allocation.
It could have been named heapAllocated or anything.

I also had to change the pool system from alloc(size) to authorize(size) + commit(size).

If you have preferred naming just tell me, or you can also edit the PR.