CORE/SERVICE_COLL: Add embedded req flag and ctx-scoped allreduce - #1347
Open
bwestheimer wants to merge 3 commits into
Open
CORE/SERVICE_COLL: Add embedded req flag and ctx-scoped allreduce#1347bwestheimer wants to merge 3 commits into
bwestheimer wants to merge 3 commits into
Conversation
The alloc path builds an id as i * 64 + pos, where i is the pool word index and pos is in [1, 64], so bit (pos - 1) of word i encodes that id. The release path instead computed map_pos = id / 64, which for any id that is a multiple of 64 names the next word: id 64 was released as word 1 bit 63, the slot for id 128. That leaks id 64 and hands out id 128 twice. Use map_pos = (id - 1) / 64 so release is the exact inverse of alloc. Expose the two pool bit helpers so the boundary behavior can be tested directly, and add a gtest that round-trips ids across every word boundary and checks no residue is left in the pool.
Move the ucc_assert(id >= 1) guard to the top of ucc_team_id_pool_set_bit so invalid ids are rejected before the (id - 1) subtraction; a value of 0 or INT_MIN would cause signed overflow UB before the assert fires.
Add an 'embedded' flag to ucc_service_coll_req_t. When set, ucc_service_coll_finalize skips ucc_free(req) so the request can live in caller-owned storage instead of being heap allocated. The team agreement vote embeds its request directly in ucc_team_t and relies on this. Add ucc_service_allreduce_ctx, which runs an allreduce over a pre-materialized subset of context endpoints using the context-level service team. The existing ucc_service_allreduce resolves subset ranks through team->ctx_map, which is not yet built while a team is being created. The new entry point maps subset indices straight to context ranks and routes through ctx->service_team, so the agreement vote can run before ctx_map exists.
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
1 participant
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.
What
Add
embeddedflag toucc_service_coll_req_tand a newucc_service_allreduce_ctx()function.When
embeddedis set,ucc_service_coll_finalizeskipsucc_free(req)so callers can embed the request inside a larger struct rather than heap-allocating it.ucc_service_allreduce_ctx()runs a BAND allreduce over a caller-supplied ctx-rank subset via the context-level service team, without requiring a built teamctx_map.Why
Part of the communicator caching series (see #1346 for context). The cache agreement vote runs during
ucc_team_create_postbefore the new team'sctx_mapis available. The existingucc_service_allreducerequires a built map; this variant takes a direct ctx-rank list. Theembeddedflag lets the vote request live insideucc_team_twith no separate allocation.Depends on #1346.