feat: integrate OAHSet#7246
Conversation
There was a problem hiding this comment.
Pull request overview
Integrates OAHSet as an optional runtime-selected backing implementation for dense SETs (kEncodingStrMap2) behind a new --use_oah_set flag, and adapts server paths (commands, RDB/journal, deletion/defrag) to operate over either StringSet or OAHSet via VisitSet() dispatch.
Changes:
- Introduces
--use_oah_setand a global snapshot (dfly::g_use_oah_set) to select the concrete dense-set type at startup. - Updates SET-related operations (iteration, expiry touch, serialization, conversion from intset/listpack) to be generic over
StringSet/OAHSet. - Extends core memory accounting/defrag/free paths to handle the new dense-set implementation.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/zset_family.cc | Uses VisitSet to set dense-set time before iterating members for ZSET ops involving SET inputs. |
| src/server/test_utils.cc | Plumbs FLAGS_use_oah_set into g_use_oah_set for tests. |
| src/server/set_family.h | Updates ConvertToStrSet API to return a type-erased pointer for StringSet/OAHSet. |
| src/server/set_family.cc | Adds flag definition, generic dense-set wrappers/helpers, and supports building dense sets as OAHSet. |
| src/server/server_family.cc | Updates SERVER SHRINK to support shrinking OAHSet-backed SETs. |
| src/server/rdb_save.cc | Serializes SETs via VisitSet, including expiry-aware type selection. |
| src/server/rdb_load.cc | Loads dense SETs into either StringSet or OAHSet depending on g_use_oah_set. |
| src/server/journal/cmd_serializer.cc | Disables/restores lazy expiry for dense SET serialization via VisitSet. |
| src/server/generic_family.cc | Ensures dense SET expiry time is touched before container iteration via VisitSet. |
| src/server/dfly_main.cc | Wires FLAGS_use_oah_set into dfly::g_use_oah_set at startup. |
| src/server/db_slice.cc | Extends async deletion logic to support OAHSet via templated AsyncDeleter. |
| src/server/container_utils.cc | Iterates dense SETs via VisitSet and common Key(it) extraction. |
| src/server/collection_family_fallback.cc | Updates fallback signature for ConvertToStrSet. |
| src/core/oah_set_test.cc | Renames alloc accounting APIs in tests to ObjMallocUsed / SetMallocUsed. |
| src/core/oah_set.h | Adds lazy-expiry during iteration, exposes malloc-used APIs, and introduces VisitSet/Key helpers + g_use_oah_set. |
| src/core/compact_object.cc | Updates free/size/defrag logic for dense SETs to dispatch via VisitSet. |
| size_t bucket_bytes_before = ds->BucketCount() * sizeof(void*); | ||
| size_t optimal_size = std::max(size_t(8), absl::bit_ceil(ds->UpperBoundSize())); | ||
| ds->Shrink(optimal_size); | ||
| size_t bucket_bytes_after = ds->BucketCount() * sizeof(void*); |
There was a problem hiding this comment.
bucket_bytes_before/after is computed as BucketCount() * sizeof(void*), which is not valid for OAHSet (buckets are OAHEntry, not pointers). This makes the SHRINK reply (freed bytes) incorrect under --use_oah_set; consider using SetMallocUsed() before/after shrink (available on both DenseSet and OAHSet) or otherwise accounting based on the actual bucket element size.
🤖 Augment PR SummarySummary: Adds an optional Changes:
Technical Notes: 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
augment review |
Summary: This PR integrates OAHSet as an optional dense backing store for kEncodingStrMap2 SETs behind a new --use_oah_set flag.
Changes:
the results of use_oah_set=true are in fake PR #7247