Refine mgmt emitter: replace isPrefix with getSharedSegmentCount in resource-detection#56185
Refine mgmt emitter: replace isPrefix with getSharedSegmentCount in resource-detection#56185
Conversation
…esource-detection - Add getSegmentCount helper to utils.ts to avoid redundant path splitting - Replace isPrefix + getSharedSegmentCount combo in resource-detection.ts with direct getSharedSegmentCount + getSegmentCount calls - Update isPrefix itself to use getSegmentCount instead of inline splits - Remove isPrefix import from resource-detection.ts (no longer needed) This eliminates redundant string splitting operations where isPrefix was called followed by getSharedSegmentCount on the same paths. Addresses feedback from PR #56180 (discussion r2791991621).
live1206
requested review from
ArcturusZhang,
JoshLove-msft,
jorgerangel-msft and
m-nash
as code owners
github-actions
bot
added
CodeGen
There was a problem hiding this comment.
Pull request overview
Refines the management-plane emitter’s legacy resource detection logic to reduce redundant path splitting when performing prefix/segment comparisons for parent/child resource matching.
Changes:
- Added a
getSegmentCount()utility and updatedisPrefix()to use it. - Updated
resource-detection.tsto usegetSharedSegmentCount()+getSegmentCount()directly instead of combiningisPrefix()with additional segment counting. - Removed the unused
isPrefiximport fromresource-detection.ts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| eng/packages/http-client-csharp-mgmt/emitter/src/utils.ts | Introduces getSegmentCount() and refactors isPrefix() to use segment counts rather than duplicating split/filter logic inline. |
| eng/packages/http-client-csharp-mgmt/emitter/src/resource-detection.ts | Replaces isPrefix usage with explicit shared/segment-count checks to avoid redundant splitting during resource parent detection. |
| const parentSegmentCount = getSegmentCount(existingParentPath); | ||
| const sharedCount = getSharedSegmentCount( | ||
| existingParentPath, | ||
| operationPath | ||
| ); | ||
| if ( | ||
| sharedCount === parentSegmentCount && | ||
| sharedCount <= getSegmentCount(operationPath) | ||
| ) { |
There was a problem hiding this comment.
Inside the for (const [existingKey] ...) loop, getSegmentCount(operationPath) is recomputed for every candidate check. Since operationPath doesn’t change within the loop, compute its segment count once before iterating (or at least once per processMethod call) and reuse it to avoid repeated split/filter work in this hot path.
ArcturusZhang
left a comment
ArcturusZhang
left a comment
There was a problem hiding this comment.
this looks fine, but if we really want to solve this on this end, the best way is to have a class describing request paths, which could keep tracking its own segments so that we do not have split a string all the time
Description
Addresses feedback from #56180 (comment)
The isPrefix function in resource-detection.ts was being called alongside getSharedSegmentCount, leading to redundant path splitting operations. Since isPrefix internally calls getSharedSegmentCount, this resulted in the same paths being split multiple times.
Changes
Testing