search

WebReflection / coincident

An Atomics based Proxy to simplify, and synchronize, Worker related tasks.
MIT License
189 stars 3 forks source link

Coincident fails when a proxied function returns a large Uint8Array #29

Closed e-nikolov closed 11 months ago

e-nikolov commented 11 months ago

I am not sure if this is a bug or a browser limitation but if I use coincident to proxy a function that returns a Uint8Array of size 7173 or larger I get RangeError: Maximum call stack size exceeded on Chrome. It works fine until that magic number.

On Firefox the magic number is 51111, at which point I get RangeError: too many function arguments

I also noticed that if I return a Uint32Array instead of a Uint8Array, the magic numbers increase to 4 7173 and 4 51111.

Example:

index.html:

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <script type="module">
        console.log('main');
        import coincident from '../../es.js';
        const proxy = coincident(new Worker('./worker.js', {type: 'module'}));
        proxy.func = () => {
          console.log(`func: `);

          // let buf = new ArrayBuffer(7172);     // works on Chrome
          let buf = new ArrayBuffer(7173);        // fails on Chrome
          // let buf = new ArrayBuffer(51110);    // works on Firefox
          // let buf = new ArrayBuffer(51111);    // fails on Firefox
          return new Uint8Array(buf);
        }
      </script>
</head>

</html>

worker.js

console.log('worker.js');

import coincident from '../../es.js';
const proxy = coincident(self);

(async () => {
    let x = proxy.func();
    console.log("len: ", Object.keys(x).length);
})();
e-nikolov commented 11 months ago

I think it fails here, but there might be other places as well:


 () => parse(fromCharCode(...new Uint16Array(sb.buffer).slice(0, length)))

It happens because there is a limit on the number of arguments that can be passed to a function. In this case, if sb.buffer has more than 65536 elements, fromCharCode will get too many arguments because of the spread syntax (...).

Relevant Blog, MDN and WebKit Bug Report

WebReflection commented 11 months ago

@e-nikolov I blogged about this more than 12 years ago: http://webreflection.blogspot.com/2011/07/about-javascript-apply-arguments-limit.html

last time I've checked, the fromCharCode official test for browsers used a Uint8Array over a png so I assumed (wrongly, apprently) the arity issue was long solved by now ... of course I was wrong, if your test is correct.

e-nikolov commented 11 months ago

From my tests, the limit is 500 000 arguments for Firefox and for Chromium it's 65536, but for some reason, it sometimes grows to a number between 125611 and 125644 based on previous calls.

WebReflection commented 11 months ago

JIT gotta JIT things ... don't worry, I have your use case to test against, and enough understanding to fix this reliably ... just bear with me a little while, thanks!

e-nikolov commented 11 months ago

A related issue is that currently it's only possible to supply a custom serializer to string. It would be nice if it was also possible to supply a custom serializer that packs to binary data like Cbor or MessagePack.

WebReflection commented 11 months ago

actually the serlializer in PyScript is a structuredClone compatible one, so that recursion is safe to pass around ... other serlializers might not be able to do so, but in coincident you are in charge of passing your own parse and stringify and it's explained in the README so your issue is really a no-issue as it's already addressed by the API.

WebReflection commented 11 months ago

also please let's keep the dicussion relevant to the bug, feel free to open a discussion or another bug if needed, thanks.

e-nikolov commented 11 months ago

The issues are sort of related because the call to fromCharCode() that causes this bug returns a string that is passed to the parse() function. So I can't supply a custom parse() function that expects an array of bytes, I have to pass a function that takes a string and then converts it to bytes, which is a bit of a waste because fromCharCode() was just used to turn a byte array into string.

If it was possible to avoid the stringification, it might solve both issues.

WebReflection commented 11 months ago

This has been tested and fixed with latest coincident and it's reflected to the latest polyscript module too.

For PyScript, we need some other MR to go in, but I'll update that too for sure :wave:

WebReflection commented 11 months ago

If it was possible to avoid the stringification, it might solve both issues.

I need to parse something and that's a string because serialization is the only thing that survives postMessage (edit beside buffers, of course, but here buffers need to return a JS value in a way or another) ... I am not sure what you are after here but this issue has been solved, so please open a new one where we can discuss other gotchas ... the parse as not string might be hostile to the library because on parse both JSON and JSON like utilities expect a string there.

WebReflection commented 11 months ago

P.S. I could add a parseBinary chek to the bootstraping object and give you the Uint16Array there as it lands ... would that work? Still please let's have this discussion elsewhere, thanks!