policy: reduce allocs by avoiding use of interface types, retire MapStateOwners #36798
Conversation
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
|
/test |
22a844a to
19fcccf
Compare
|
small cleanup |
|
/test |
19fcccf to
5a0ec9d
Compare
|
/test |
5a0ec9d to
7aad0a0
Compare
|
/test |
7aad0a0 to
5c067cd
Compare
|
/test |
|
/test |
44fdb85 to
f788f4a
Compare
|
/test |
f788f4a to
c06c59c
Compare
Use Key[K] as a type constraint, rather than the type of the stored key, or the key parameter. With this change the generic type K within the type and function parameters is the actual key type, rather than an interface, avoiding indirection and taking less space. Nil checks can no longer performed on the key parameter as it is no longer an interface. If 'K' is a pointer type, its implementations of Key[K] interface functions must accept nil receiver and perform nil checks itself. If 'K' is small it should be passed by value for better performance as this avoids memory indirections. This change alone speeds up the CIDR deny policy benchmark (BenchmarkRegenerateCIDRDenyPolicyRules): by ~12%, reduces the memory needed by ~10%, and reduces the number of heap allocations by ~50%. This change is also needed for removal of the Iterator interface in a later commit. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Remove the Iterator interface and return the implementation of the iterator directly by value. This removes a heap allocation for each iteration, as the iterator is allocated in the stack of the calling function. This change speeds up the CIDR deny policy benchmark (BenchmarkRegenerateCIDRDenyPolicyRules) by ~12%, reduces the memory needed by ~13%, and reduces the number of heap allocations by ~65%. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Fix mispelling of 'mapStateEntry' and commenta in LPMAncestors(). Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
The set of MapStateEntry owners was added to keep track of which selectors need a specific MapStateEntry with a specific identity to be present, so that when an identity is incrementally deleted from one selector, the MapStateEntry is actually deleted when the identity is removed from all the selectors that had it. Now that selector cache is transactional we can rely on any deleted identity to be removed from all selectors within the same incremental map change. Thus we no longer need to track the MapStateOwners. This change speeds up the CIDR deny policy benchmark (BenchmarkRegenerateCIDRDenyPolicyRules) by ~10%, reduces the memory needed by ~25%, and reduces the number of heap allocations by ~8%. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
The host identity can be mutated, e.g., when the kube-apiserver is removed from the local host. When this happens, the 'reserved:kube-apiserver' label is removed from the host identity (1), causing any selectors selecting the 'reserved:kube-apiserver' label to no longer select the host identity (1). Since we no longer have any reference counting (such as owner tracking) in the policy map state, we must block the incremental deletion of any such mutated identities to keep traffic to/from that identity allowed in case some other selector may be still allowing such traffic. For this strategy to work, the entity mutating the host identity must also trigger forced policy regenerations for all endpoints so that the policy map entries with the host identity are removed if actually not required due to any other selectors. To implement this in a generic was we add a "mutated" boolean return value to SelectorCache.UpdateIdentities function. This function is already explicitly aware of such mutated identities. Instead of informing the SelectorCache user's of a deleted identity in this case, we simply just update the cached selector that no longer selects the mutated identity and return "mutated = true" to inform the caller to trigger forced policy updates on all endpoints when this happens. IPCache metadata logic is changed to make note of the new "mutated" return value and trigger policy updates when 'true' is returned. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Reduce allocations and memory use by preallocating the mapstate map with the capacity of the size of the previous map. This minimizes allocations for policy updates, but also means that the policy map shrinks only after the 2nd policy recompute. This change speeds up the CIDR deny policy benchmark (BenchmarkRegenerateCIDRDenyPolicyRules) by ~9%, reduces the memory needed by ~44%, and reduces the number of heap allocations by ~4%. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Remove unused 'epLabels' parameter from 'policyDistillery.distillPolicy()'. Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
6b191c4 to
bd65c20
Compare
|
Fixed conflict with main. |
|
/test |
45 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Reduce allocations in policy map computation by:
Change the
container/bitlpm.Trietype to useKey[K]as a type constraint instead of the stored key type. This allows the actual key type to be stored by value, reducing needed memory allocations and indirection.Remove the Iterator interface type and return the iterators by value so that they can be stored in the caller's stack instead of in the heap. This change makes the use of the explicit iterators faster than the callback based alternatives.
Remove the tracking of
MapStateOwnersmade redundant by the change of FQDN identities to not mutate their labels (#32769)reserved:hostidentity is still possible and is handled by not issuing incremental deletion for the change and informing the caller (ipcache) to trigger forced policy regenerations in this case.Preallocate policy MapState map using the size of the previous policy map.
These changes speed up the CIDR deny policy benchmark
(
BenchmarkRegenerateCIDRDenyPolicyRules) by ~36%, reduces the memoryneeded by ~67%, and reduces the number of heap allocations by ~85%.
These changes bring the
BenchmarkRegenerateCIDRDenyPolicyRulesin v1.17 to be ~10000x faster than in v1.16, with 99,5% reduction in memory consumption (99,6% reduction in the number of heap allocations).BenchmarkRegenerateCIDRPolicyRules(without the deny rules) is now ~3.6x faster than v1.16, with 80% reduction in memory consumption (99% reduction in the number of heap allocations).