[PR] [CELEBORN-2395] Reduce allocations in Utils.split* methods on worker hot path [celeborn]

["yew1eb (via GitHub)" <gi...@apache.org>]

yew1eb opened a new pull request, #3771:
URL: https://github.com/apache/celeborn/pull/3771

   ## Description
   
   `Utils.splitShuffleKey` / `splitPartitionLocationUniqueId` / `splitAttemptKey` used `String.split("-")` (regex + `String[]`) plus `dropRight(1).mkString("-")` (array copy + `StringBuilder` + new `String`) to slice the trailing segment. Since the trailing segment (`shuffleId` / `epoch` / `attemptId`) is always the last `-`-delimited value, a single `lastIndexOf('-')` / `indexOf('-')` plus two `substring` calls suffices — zero regex, zero intermediate array, zero `StringBuilder`.
   
   `splitShuffleKey` in particular sits on the per-push-message hot path (`recordAppActiveConnection` + `checkAuth`).
   
   ### Correctness note
   
   `applicationId` may itself contain `-` (e.g. Spark's `application_<timestamp>_<id>`), so `lastIndexOf` (not `indexOf`) is required for `splitShuffleKey` to slice only the trailing `shuffleId`. For `splitPartitionLocationUniqueId` / `splitAttemptKey`, both segments are `Int` and never contain `-`, so `lastIndexOf` / `indexOf` are equivalent and safe. Pure refactor, no behavioral change.
   
   ## Evidence
   
   Production async-profiler CPU flame graph on `celeborn-worker` (80,611 samples; true stack-reconstruction, self sum = 100%). The frame's self cost is near zero — the cost is inclusive, in the JDK methods it calls. By call site (inclusive):
   
   | call site | inclusive |
   |---|---|
   | `WorkerSource.recordAppActiveConnection` → `splitShuffleKey` | 2.95% |
   | `PushDataHandler.checkAuth` → `splitShuffleKey` | 1.27% |
   | total | ~4.23% |
   
   Both are on the push path. The 2.95% one is default-on per-app metrics (disable via `celeborn.metrics.worker.appLevel.enabled=false`); the ~1.27% checkAuth path is always-on.
   
   ## How was this patch tested?
   
   - New unit tests in `UtilsSuite` covering `splitShuffleKey` (incl. `applicationId` containing `-`, plus a `makeShuffleKey` round-trip), `splitPartitionLocationUniqueId`, and `splitAttemptKey`.
   - `./build/mvn -pl common -Dtest=UtilsSuite test` — all `split*` tests pass. (Two unrelated `CelebornConfSuite` failures — `Fallback to parent module's config...` and `rpc_service and rpc_client...` — are pre-existing on a clean `upstream/main`; verified by stashing this change and re-running.)
   - `./build/mvn -pl common spotless:check` — clean.
   
   Closes #CELEBORN-2395.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2395] Reduce allocations in Utils.split* methods [celeborn]

["SteNicholas (via GitHub)" <gi...@apache.org>]

SteNicholas closed pull request #3771: [CELEBORN-2395] Reduce allocations in Utils.split* methods
URL: https://github.com/apache/celeborn/pull/3771


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [CELEBORN-2395] Reduce allocations in Utils.split* methods [celeborn]

["SteNicholas (via GitHub)" <gi...@apache.org>]

SteNicholas commented on PR #3771:
URL: https://github.com/apache/celeborn/pull/3771#issuecomment-5161744351

   Thanks. Merged to main(v1.0.0) and branch-0.7(v0.7.0).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org