Cursor.each should work in batches #804
Conversation
| Higher memory usage would be normal in this case as you're keeping around more items in each stack frame. process.nextTick with nextObject means the stack frame is tiny for each. But if you have a couple of thousands of objects in each process.nextTick GC will have to do more work collecting them as the last graph shows pretty well. More memory usage pr say is not a problem but it might be worth benchmarking it with higher concurrency (like 50-100 clients running cursors on the same size of data) and see what the memory impact is. I have been thinking about looking into using trampolining instead of process.nextTick but have had no luck getting it to work. By the way process.nextTick is not necessarily a bad thing. Think of it as yielding to the event loop so it can schedule other concurrent in flight work. |
| I have updated my benchmark to run with concurrency. You can also grab the code here to run the tests yourself. A few more pretty graphs with concurrency=100
The memory numbers are now actually better with my patch. I also noticed that I’m actually using Cursor.toArray() instead of .each() which surely adds some memory overhead. Regarding process.nextTick: |
| Since my use case involves Cursor.toArray, why not use batches there too instead of delegating to .each… |
| I can't seem to get the code you provided to run, I assume it's using some stuff I cannot see in your environment. Can your provide some simple instructions on how to run it and generate the graphs as I'm interested in just playing with it a bit before taking a decision on the pull request. So far it looks good though but I need to test with varying sizes of documents being returned as well as concurrency. |
| I’ve added a README: Swatinem/mongobench@cd43bb0 Should be straight forward, just copy/symlink mongodb into $directory and run the nodejs script and save the output to a .tsv file. |
| Had to back out this checkin due to problems with replicasets when it was in, will investigate it again after christmas |
2 participants
Currently, Cursor.each uses a really inefficient process.nextTick -> nextObject() -> recurse to self
loop.
This patch avoids this behavior and rather loops over a batch directly, providing a nice speedup up to 2x depending on workload.
I’ve run
make testand its passing without issues.The benchmarks I have done are available at Swatinem/mongobench. I can observe the following speedups:
(jitter graph is in log scale)
The patched version however uses more memory:
I might need some help fixing that.