chore: minor refactorings around dense_set deletions #4390
Conversation
| @@ -76,29 +76,38 @@ void DeallocateAtRandom(size_t steps, std::vector<void*>* ptrs) { | |||
| } | |||
| } | |||
|
|
|||
| static void InitThreadStructs() { | |||
| auto* tlh = mi_heap_get_backing(); | |||
There was a problem hiding this comment.
factored out the code below into dedicated functions. no changes here
| @@ -168,14 +168,14 @@ auto DenseSet::PopPtrFront(DenseSet::ChainVectorIterator it) -> DensePtr { | |||
| return front; | |||
| } | |||
|
|
|||
| uint32_t DenseSet::ClearInternal(uint32_t start, uint32_t count) { | |||
| uint32_t DenseSet::ClearStep(uint32_t start, uint32_t count) { | |||
There was a problem hiding this comment.
renamed the function and made it public.
romange
changed the title
Done as a preparation to introduce asynchronous deletions for sets/zsets/hmaps. 1. Restrict the interface around DbSlice::Del. Now it requires for the iterator to be valid and the checks should be explicit before the call. Most callers already provides a valid iterator. 2. Some minor refactoring in compact_object_test. 3. Expose DenseSet::ClearStep to allow iterative deletions of elements. Signed-off-by: Roman Gershman <[email protected]>
| if (!IsValid(it)) | ||
| continue; | ||
|
|
||
| db_slice.Del(op_args.db_cntx, it); | ||
| ++res; |
There was a problem hiding this comment.
nit
if (IsValid(it)) {
db_slice.Del(op_args.db_cntx, it);
++res;
}
What is asynchronous deletions for sets/zsets/hmaps? how/when will it be used? |
|
@adiholden , asynchronous/iterative deletions of huge sets is similar to So I think its interface is better now and I wanted to separate these fixes from the async PR. |
| @@ -1169,10 +1169,10 @@ void HSetFamily::HRandField(CmdArgList args, const CommandContext& cmd_cntx) { | |||
| } | |||
| } | |||
|
|
|||
| if (string_map->Empty()) { | |||
| auto it_mutable = db_slice.FindMutable(db_context, key, OBJ_HASH); | |||
| it_mutable->post_updater.Run(); | |||
There was a problem hiding this comment.
you removed in few places in this pr calls to post_updater.Run()
and added a comment that it will run immediately.
how will it run? I believe that in the current interface we need to run it before we call del, not so good approach but I think that you break something here if you remove this
There was a problem hiding this comment.
ItAndUpdater is returned by FindMutable as a temporary and is destructed right after the call. ItAndUpdater object contains post_updater that will run during the destruction. It's not explicit. hence the comment that that explains it.
There was a problem hiding this comment.
I see now.. thats very confusing
There was a problem hiding this comment.
I actually borrowed it from Shachar's code from another place. I find it acceptable if it has a comment :)
Done as a preparation to introduce asynchronous deletions for sets/zsets/hmaps.