search

numbers / numbers.js

Advanced Mathematics Library for Node.js and JavaScript
Apache License 2.0
1.77k stars 167 forks source link

min / max consumes O(n) memory #136

Closed timanovsky closed 9 years ago

timanovsky commented 9 years ago

They are implemented using JS apply() thus copying an array into the stack frame, on large arrays it causes out of memory and crash. Easy to reproduce in nodejs.

Dakkers commented 9 years ago

I'm assuming your suggestion is to rewrite the functions using a native for-loop?

timanovsky commented 9 years ago

I tried very simple tests in nodejs. It looks like reduce is about 2X slower than apply; and for loop is 10X faster than reduce and 5X faster than apply.

Dakkers commented 9 years ago

@timanovsky what size arrays were you using? I was able to use arrays of size 10,000,000.

timanovsky commented 9 years ago

About 100 000. Probably it depends on node platform or version, I was testing with 10.28 under MacOS

Best regards, Alexey Timanovsky.

On 3 March 2015 at 01:35, Dakota St. Laurent notifications@github.com wrote:

@timanovsky https://github.com/timanovsky what size arrays were you using? I was able to use arrays of size 10,000,000.

— Reply to this email directly or view it on GitHub https://github.com/numbers/numbers.js/issues/136#issuecomment-76841844.

devinjacobson commented 9 years ago

Dont think this is an issue in this numbers library.
min consumes no addl. memory. Arrays should be passed as reference as in other JS engines.
Perhaps a problem on Mac's nodejs.

Dakkers commented 9 years ago

unfortunate. nothing can be done on our end, then