refactor: extract shard allocator into a dedicated crate#3578
Open
jayakasadev wants to merge 6 commits into
Open
refactor: extract shard allocator into a dedicated crate#3578jayakasadev wants to merge 6 commits into
jayakasadev wants to merge 6 commits into
Conversation
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3578 +/- ##
=============================================
- Coverage 74.35% 60.15% -14.20%
Complexity 937 937
=============================================
Files 1257 1256 -1
Lines 130671 118917 -11754
Branches 106537 94829 -11708
=============================================
- Hits 97156 71532 -25624
- Misses 30434 44193 +13759
- Partials 3081 3192 +111
🚀 New features to boost your workflow:
|
Moving the allocator into server_common pulled hwlocality into nearly the entire workspace, since almost every crate depends on server_common. That broke the aarch64-musl build: simulator links with -nodefaultlibs and the vendored hwloc drags in glibc's libm.a, failing on an undefined reference to errno. Keep CpuAllocation and NumaConfig in server_common::sharding, which configs re-exports and which carry no hwloc, and move the hwloc-backed ShardAllocator into a new shard_allocator crate that only server and server-ng depend on. This restores the confinement of hwloc to those two binaries that master had. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
/request-review @hubcio |
The shard allocator extraction parked CpuAllocation/NumaConfig in server_common, which forced a serde dependency onto a runtime crate and coupled the lightweight allocator to server_common's heavy tree. Move the two pure config types into a dedicated cpu_allocation leaf crate that depends only on serde, re-export them from configs to keep the configs::sharding path stable, and point shard_allocator at the leaf crate. Also drop the whole-crate re-export shim in server::lib in favor of direct paths, and document the new crate's public surface. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Vendored hwloc references cbrt, forcing the linker to resolve -lm. musl folds math into libc and rust's musl sysroot ships no libm.a, so -lm fell through to the host glibc libm.a, whose cbrt needs glibc-only __frexp/__ldexp symbols absent on musl, breaking the aarch64-musl build. Drop an empty libm.a on the search path (musl-gated build script) so -lm resolves to nothing and cbrt is satisfied later by musl's own libc. This fixes the root cause independent of crate link ordering. Co-authored-by: Cursor <cursoragent@cursor.com>
785f528 to
9c53a6f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR address?
Relates to #3315
Rationale
core/server-ngfrom the legacycore/servercrateWhat changed?
allocator moved to
shard_allocatorcrate that onlyserverandserver-ngdepend onserver_common,hwlocalityleaked everywhere and broke theaarch64-muslbuildsimulatorlinks with-nodefaultlibshwlocdrags in glibc'slibm.a, failing on an undefined reference toerrnoserver-nghad to importservercrate to use shard allocator (server::shard_allocator) before thisCpuAllocation/NumaConfigmoved to a dedicatedcpu_allocationleaf crateserde, nohwlocshard_allocatordepends oncpu_allocation(notconfigsorserver_common), so the allocator stays light andhwlocnever leaks back intoconfigsconfigs::shardingre-exports them to keep theconfigs::sharding::*path stable for existing callersallocator lives in its own
shard_allocatorcrate that onlyserverandserver-ngdepend onserver_common,hwlocalityleaked everywhere and broke theaarch64-muslbuildsimulatorlinks with-nodefaultlibshwlocdrags in glibc'slibm.a, failing on undefined references to glibc-internal math symbolsserver-ngpreviously had to depend on theservercrate just to reach the allocator (server::shard_allocator)CpuAllocation/NumaConfigmoved to a dedicatedcpu_allocationleaf crateserde, nohwlocshard_allocatordepends oncpu_allocation(notconfigsorserver_common), so the allocator stays light andhwlocnever leaks back intoconfigsconfigs::shardingre-exports them to keep theconfigs::sharding::*path stable for existing callersshard_allocatorcarries a tiny, musl-gated build script that drops an emptylibm.astub on the link search pathhwlocreferencescbrt, which forces the linker to resolve-lm; musl folds math intolibc, so rust's musl sysroot ships nolibm.aand-lmwould otherwise fall through to glibc'slibm.a(whosecbrtneeds glibc-only__frexp/__ldexp, absent on musl)-lmharmlessly socbrtresolves from musl's ownlibc, fixing the link independent of crate orderingLocal Execution
AI Usage
rg 'server::shard_allocator' core/server-ng/is empty and that nothing was broken by the move