Use StringComparisonAdaptor as key to allow lookup with string_view#628
Open
ahmed0mousa wants to merge 4 commits into
Open
Use StringComparisonAdaptor as key to allow lookup with string_view#628ahmed0mousa wants to merge 4 commits into
ahmed0mousa wants to merge 4 commits into
Conversation
…ment maps Replace std::string keys with score::memory::StringComparisonAdaptor StringComparisonAdaptor stores the string but allows lookups with std::string_view without memory allocations, avoiding unnecessary string construction at call sites. Also updates ContainsEvent/ContainsField/ContainsMethod and GetServiceElementInstanceDeployment/GetServiceElementId to take std::string_view instead of const std::string& now that the map key supports it. Issue: SWP-270173
Update GetQueueSize, GetTypeErasedElementInfo, and GetServiceElementId callers
to take std::string_view instead of const std::string&.
Remove unnecessary std::string{...} constructions from std::string_view parameters
at all call sites in the plumbing layer. This avoids heap allocations when looking
up service elements by name.
Issue: SWP-270173
Call GetAsStringView() when extracting map keys (now StringComparisonAdaptor) for use as std::string_view, e.g. when logging, inserting into std::set<string_view>, or passing to functions expecting string_view. Issue: SWP-270173
…nvertToTracingServiceInstanceElement path Issue: SWP-270173
ahmed0mousa
requested review from
LittleHuba,
bemerybmw,
castler,
crimson11,
hoe-jo and
limdor
as code owners
limdor
requested changes
Jul 1, 2026
| using EventIdMapping = std::unordered_map<std::string, EventIdType>; | ||
| using FieldIdMapping = std::unordered_map<std::string, FieldIdType>; | ||
| using MethodIdMapping = std::unordered_map<std::string, MethodIdType>; | ||
| using EventIdMapping = std::unordered_map<score::memory::StringComparisonAdaptor, EventIdType>; |
Contributor
There was a problem hiding this comment.
Using StringComparisonAdapter here make very hard to reason about lifetimes. It could be that you own the string and it could be that you do not know the string. If this is only for lookup, is it not possible with custom comparison?
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
2 participants
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.
No description provided.