-
-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LRUCache throws "Invalid Array length" error on Node.js 12.22.1 for capacity=4294967295 #167
Comments
This appears to be limitation put forward by the runtime version. When mnemonist/utils/typed-arrays.js Lines 26 to 39 in 825538a
But it throws error on Node.js 14.x as follows: Codeconst LRUCache = require("mnemonist/lru-cache");
new LRUCache(Math.pow(2, 32)); Output/Users/trivikr/workspace/lru-cache/node_modules/mnemonist/lru-cache.js:45
this.forward = new PointerArray(capacity);
^
RangeError: Invalid typed array length: 4294967296
at new Uint32Array (<anonymous>)
at new LRUCache (/Users/trivikr/workspace/lru-cache/node_modules/mnemonist/lru-cache.js:45:18)
at Object.<anonymous> (/Users/trivikr/workspace/lru-cache/index.js:3:1)
at Module._compile (internal/modules/cjs/loader.js:1068:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
at Module.load (internal/modules/cjs/loader.js:933:32)
at Function.Module._load (internal/modules/cjs/loader.js:774:14)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
at internal/main/run_main_module.js:17:47 |
According to ECMAScript Spec on TypedArray Objects:
From StackOverflow answer on Do ArrayBuffers have a maximum length? |
Should LRUCache fall back to using an Array if PointerArray for provided capacity throws RangeError? Verified that Array of size 4294967295 can be created in Node.js v12.22.1: $ node
Welcome to Node.js v12.22.1.
Type ".help" for more information.
> new Uint32Array(4294967295);
Uncaught RangeError: Invalid typed array length: 4294967295
at new Uint32Array (<anonymous>)
> new Array(4294967295);
[ <4294967295 empty items> ] |
Should LRUCache add Array of size 4294967296 can't be created in Node.js v14.17.0: $ node
Welcome to Node.js v14.17.0.
Type ".help" for more information.
> new Float64Array(4294967296);
Uncaught RangeError: Invalid typed array length: 4294967296
at new Float64Array (<anonymous>)
> new Array(4294967296);
Uncaught RangeError: Invalid array length |
Yes unfortunately JS cannot handle indices > 32 bits. Sometimes we can cheat using the 53 bits of the float mantissa but not here. So I guess you can add an upper limit to capacity indeed. If I was cheeky I could also tell you it feels weird to be attempting to create such a large LRU cache since usually they are used to save memory :) It is possible to create composite arrays to index up to 53 bits but the perf cost will start becoming very high to support this. I am wondering whether throwing the error could be thrown from here instead of returning a |
Yup, any suggestion on what the error message should be? throw new Error('mnemonist: Pointer Array of size > 4294967295 is not supported.'); Should LRUCache just rethrow the same error, or throw a different custom error like this? throw new Error('mnemonist/lru-cache: LRUCache of size > 4294967295 is not supported.'); |
Your message looks good to me. It could be maybe tell that JavaScript is the culprit :)
It would be nice, but it will be a drag to rethrow as a custom error in every class that needs it I think. I guess a generic error would be less hassle in this particular case. |
PR to throw error if array size > 4294967295 in getPointerArray is posted at #168 I'll keep this issue open to discuss solution if the platform doesn't support array length for TypedArrays.
|
It's right that we could try and use regular arrays in this case. In which case I suspect they are limited to 53 bits indices? One issue here though is obviously that the array should usually be zeroed completely before being used in most use cases to remain transparent with typed array usage. Another issue is of course performance. For full precisions floats, JS arrays on V8 display roughly the same performance as a static preallocated JS dynamic array (but this might change if the number of items is greater than 32 bits). So I think I would be against having this Array fallback without proprer documentation explicitly describing the caveats. Another way to discuss this issue is: do you have actual use cases where you would need one of the lib structure to hold an amount of data that overflows JS TypedArrays capacity? In which case, maybe it could be useful to scale only some of the data structures to support up to 53 bits of indexing (I don't think we can use BigInt to index arrays in JS yet?) using typed arrays (by splitting the data layer in 2 separate arrays). |
Describe the issue
LRUCache throws "Invalid array length" error on Node.js 12.22.1 for capacity=4294967295
Steps to reproduce
Run the following code with
[email protected]
Observed behavior
No error is thrown on Node.js v14.17.0
On Node.js v12.22.1, the following error is thrown:
Expected behavior
No error should be thrown for Node.js 12.22.1
The text was updated successfully, but these errors were encountered: