-
Notifications
You must be signed in to change notification settings - Fork 36
Conversation
This comment has been minimized.
This comment has been minimized.
Sample output:
Note that I capped my CPU frequency at 70%, to prevent turbo boost and throttling from messing with the numbers. Main changes from #65:
The number of blocks is hard-coded right now, but it could be turned into a flag if we want. The nice thing about reusing I run the benchmarks with 4k blocks, and 1, 64, and 1024 concurrent goroutines. You can see that the numbers are fairly similar, while the time/op slowly increases with the number of goroutines. The numbers are also very stable; they don't change much at all running for longer with
@frrist @warpfork @Stebalien @iand I'd love your thoughts. If the approach sounds good, I can rebase the other PRs atop this one and post before/after benchmark numbers for each one of them on the original issue thread. |
Also, there's no commit message for now, but I'll write one when I move this out of draft state. The code is also pretty well documented. |
5056b98
to
f1f191b
Compare
The timings per op seem sensitive to the number of blocks which I don't have a good explanation for. For example I changed to use Maybe the default 4k setting is small enough to cause contention on the blocks within the 1s timeframe, but 64k is not. I don't know anything about the locking strategy already in place for the blockstore so I am just guessing. |
Fewer blocks will cause more collisions. Some pairs of ops like Get+Delete also gradually remove all blocks from the cache, so the smaller the number of blocks, the quicker that happens. I'm also not particularly familiar with the internal architecture or what a good default for the number of blocks should be. The original PR used one million, and that definitely feels too large - even with 1k goroutines and tens of thousands of iterations, the chances for concurrent collisions are practically none. So I felt like we needed a number of blocks that was closer to the number of concurrent goroutines. |
f1f191b
to
b3408ff
Compare
…he-bench add BenchmarkARCCacheConcurrentOps This commit was moved from ipfs/go-ipfs-blockstore@10b7bf0
No description provided.