You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR splits the resource-accounting crate into its two primary parts -- resource tracking and bounds accounting -- and moves them to saluki-common and saluki-core, respectively.
As part of the refactoring work around SubsystemIdentifier (#2029, #2048), we want to utilize SubsystemIdentifier for all things that generate structured identifiers that are attached to "components", like topology components or the environment provider and so on. Part of this is utilizing SubsystemIdentifier instead of utilizing opaque string types so that we can enforce invariants -- only certain characters in identifiers, etc -- at the type system. However, to do this for the resource accounting component registry (resource_accounting::ComponentRegistry) means depending on saluki-core where SubsystemIdentifier is defined... which ends up as a circular dependency. Doh!
In preparation of moving SubsystemIdentifier to saluki-common, we need to split out the parts of resource-accounting that saluki-common depends on, and as it so happens, there's already a natural "seam", as Claude would call it, between the tracking of resource usage (allocations, CPU) and the accounting of expected usage (memory bounds, bounds verification, etc).
This PR moves all resource tracking-related bits into saluki-common to satisfy not only its own usages of those types, but also acts as the non-circular dependency on which other crates can depend to utilize those types as well. Since memory bounds-related bits are (at least as of right now) only utilized in higher-level crates where components and such are defined, we've moved those to saluki-core as all crates that have types with defined bounds already import saluki-core. This resolves all of our dependency tensions and unlocks the ability for us to move ComponentRegistry to utilize SubsystemIdentifier.
Run ID:25cf6db1-7aa5-4d2d-acb7-28525ce3e4c8 Baseline:dbece44f · Comparison:dbb35c85 · diff
Optimization Goals: ✅ No significant changes detected
Fine details of change detection per experiment (5)
Experiments configured erratic: true are tagged (ignored) and skipped when determining which experiments regressed or improved. Experiments which are detected as erratic at runtime are tagged (erratic) to flag that the run's sample dispersion was high, but their regression / improvement signal still counts.
A change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression (is_regression: true). Improvements use the matching criteria for the improving direction. Experiments configured erratic: true (tagged (ignored)) are skipped outright; experiments detected as erratic at runtime (tagged (erratic)) still count, since that flag describes sample dispersion rather than directional certainty. The Δ mean % cell is colored accordingly: 🟢 = improvement, 🔴 = regression, ⚪ = neutral. Reduction in CPU or memory is an improvement; reduction in ingress throughput is a regression.
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
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.
Summary
This PR splits the
resource-accountingcrate into its two primary parts -- resource tracking and bounds accounting -- and moves them tosaluki-commonandsaluki-core, respectively.As part of the refactoring work around
SubsystemIdentifier(#2029, #2048), we want to utilizeSubsystemIdentifierfor all things that generate structured identifiers that are attached to "components", like topology components or the environment provider and so on. Part of this is utilizingSubsystemIdentifierinstead of utilizing opaque string types so that we can enforce invariants -- only certain characters in identifiers, etc -- at the type system. However, to do this for the resource accounting component registry (resource_accounting::ComponentRegistry) means depending onsaluki-corewhereSubsystemIdentifieris defined... which ends up as a circular dependency. Doh!In preparation of moving
SubsystemIdentifiertosaluki-common, we need to split out the parts ofresource-accountingthatsaluki-commondepends on, and as it so happens, there's already a natural "seam", as Claude would call it, between the tracking of resource usage (allocations, CPU) and the accounting of expected usage (memory bounds, bounds verification, etc).This PR moves all resource tracking-related bits into
saluki-commonto satisfy not only its own usages of those types, but also acts as the non-circular dependency on which other crates can depend to utilize those types as well. Since memory bounds-related bits are (at least as of right now) only utilized in higher-level crates where components and such are defined, we've moved those tosaluki-coreas all crates that have types with defined bounds already importsaluki-core. This resolves all of our dependency tensions and unlocks the ability for us to moveComponentRegistryto utilizeSubsystemIdentifier.Change Type
How did you test this PR?
Existing tests.
References
DADP-2