Comment by vanderZwan

Comment by vanderZwan 11 days ago

The new wording is much more nuanced, appreciated! Also the fact that you link to the actual code that broke down, making it possible to verify the claim. Less ammunition for the people who actually are skeptical of the project instead of just nitpicky like me ;)

Speaking of the code in question, it looks pretty sensible - there's a bit of low-hanging fruit where one could avoid creating redundant arrays by turning chained calls to map and filter into one for-loop and such, but that's about it.

What confuses me is that there's no point in the code where I see any recursion, here or in the other JS files that seem relevant, so how does one end up with a call stack overflow?

(not questioning that it crashed for you, it's just that I don't see any obvious flaws in the implementation, so I'm curious to lean what the "gotcha" is in case I ever bump into something like it myself)

akx 11 days ago

The stack overflow is caused by an `arr=[...]; events.push(...arr)` in `add_events`. Replacing that with `for(const x of [...]) events.push(x)` gets rid of that issue, and the JS-backed build is then snappier to search & filter than the WASM/Rust version.

Reply View 5 replies

vanderZwan 11 days ago

Oh duh, yeah using spread syntax for function calls is definitely limited to fewer than 150,000 arguments in any browser that I know of. Don't expect everyone to know that, but I sure did, stupid that I didn't spot that. Thanks for pointing it out!
(funny enough I tend to use for(let i = 0; i < arr.length; i++) loops most of the time anyway because the iterator protocol adds too much overhead for my tastes, so I wasn't likely to ever bump into this in the first place)

Reply View | 4 replies
- akx 10 days ago
  
  I would imagine browsers optimize the iterator protocol away for arrays. At least on https://jsben.ch/iJFZ4 for..of is a smidge faster than an old-school loop.
  
  Reply View | 3 replies
  
  vanderZwan 10 days ago
  
  You accidentally included the array initialization in the benchmarked code ("setup block" vs "boilerplate block"), dwarfing everything else. If we're talking about an already allocated array the difference is pretty big:
  https://jsben.ch/FgDA6
  … but in a way you're correct that this is rarely the actual bottleneck in the surrounding code. Still, "death by a thousand papercuts" and all that. Plus having old-school for-loops as a habit makes it easier to spot the true botllenecks.
  
  Reply View | 2 replies