readUtf8 uses too much memory on V8 about pbf HOT 5 CLOSED

Wow, quite a subtle bug — thanks so much for sharing! I wonder if #89 would help in that case instead of using TextDecoder (which is much slower last time I checked)?

from pbf.

Nope, push + join also crashed with OOM on the repro above (with n = 27). ☹️

from pbf.

I did some benchmarking of TextDecoder here: https://jsperf.com/textdecoder-vs-readutf8/7

Seems like readUtf8 is much faster for small payloads (i.e. no startup cost) which seems pretty important for Mapbox, but becomes radically slower for larger payloads (likely because of the V8 behavior). The crossover point is in the neighborhood of 32 for my machine.

I will probably do something like this:

function readUtf8(buf, pos, end) {
  return end - pos < 32 ? 
    originalReadUtf8(buf, pos, end) : 
    textDecoder.decode(buf.subarray(pos, ed)
}

from pbf.

@dcervelli thanks a lot for your suggestion! It worked perfectly, although we found the sweet spot to be smaller (~12 characters), which fits with Benedikt's hint here.

In particular, this fixed a huge memory leak in both Chrome & Firefox in GL JS where in certain rare cases (with lots of long strings in tiles), worker memory footprint would grow to gigabytes within seconds, whereas with this patch it's stable and stays below 20MB.

from pbf.

@dcervelli One more note — the reason why the inflection point was 32 rather than 12 is that the benchmark used ASCII-only characters; once mix in some other letters (Cyrillic etc.), the perf characteristic is very different. I think we should go with 12 so that performance is stable for all languages, and I'm also good with accepting some decoding overhead for ASCII strings of length 12-32 if it helps avoid memory leaks like the one in mapbox/mapbox-gl-js#8471. cc @ahk

from pbf.