chore: improve performance of ClearInternal #3863
Conversation
Before: `BM_Clear/elements:32000 418763 ns 418749 ns 9965` After: `BM_Clear/elements:32000 323252 ns 323239 ns 12790` Signed-off-by: Roman Gershman <roman@dragonflydb.io>
| dest.obj = link->Raw(); | ||
| dest.has_ttl = link->HasTtl(); | ||
| dest.ptr = link->next; |
There was a problem hiding this comment.
Same here second time I see this maybe worth making it a function. Actually we have the exact same boilerplate in the body of the for-loop from line 277 to 297. See https://github.com/dragonflydb/dragonfly/pull/3862/files#diff-5685b15a1d75a2c80b43908203ee74ab673f9a748f1dce35ad1f018bb656eb6aR236 with the only difference the function that gets called clone vs ObjDelete
There was a problem hiding this comment.
it's not the same code because here we also call FreeLink
| if (ptr.IsObject()) { | ||
| dest.obj = ptr.Raw(); | ||
| dest.ptr.Reset(); | ||
| } else { | ||
| dest.ptr = ptr; | ||
| dest.obj = nullptr; | ||
| } |
There was a problem hiding this comment.
You merge very similar code in https://github.com/dragonflydb/dragonfly/pull/3862/files#diff-5685b15a1d75a2c80b43908203ee74ab673f9a748f1dce35ad1f018bb656eb6aR342 maybe worth extracting it into a function with an optional callback ?
| if (len == kArrLen) { | ||
| ClearBatch(kArrLen, arr); | ||
| len = 0; | ||
| } |
There was a problem hiding this comment.
Nice batching here :) Just curious why constexpr unsigned kArrLen = 32; ? Have you played around and increased this batch limt ?
There was a problem hiding this comment.
i did yeah. it can probably be differrent for differrent CPUs but it more or less near a sweet point
Before: `BM_Clear/elements:32000 418763 ns 418749 ns 9965` After: `BM_Clear/elements:32000 323252 ns 323239 ns 12790` Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Before:
BM_Clear/elements:32000 418763 ns 418749 ns 9965After:
BM_Clear/elements:32000 323252 ns 323239 ns 12790