Skip to content

Set Insertion Rework - #4999

Merged
philipaconrad merged 4 commits into
open-policy-agent:mainfrom
philipaconrad:set-insert-rework
Aug 15, 2022
Merged

philipaconrad merged 4 commits into
open-policy-agent:mainfrom
philipaconrad:set-insert-rework

Conversation

@philipaconrad

Copy link
Copy Markdown
Member

This PR introduces lazy key slice sorting for the Set datatype, similar to what was done in #4830 (Object Insertion Rework). After this change, Set.Add() will be an O(1) operation, and sorting of the Set type's key slice will be delayed until just-before-use, identically to how lazy key slice sorting is done for the Object type.

This will move the sorting overhead from construction-time for Sets over to eval-time, allowing much more efficient construction and use of enormous (500k+ item) Sets. In small-scale tests, this appears to be a performance-neutral change overall, while dramatically improving performance for the "large set" edge case.

This PR also introduces a 2x new benchmarks for Sets, to keep parity with what we introduced for Objects:

  • BenchmarkSetCreationAndLookup (Identical to BenchmarkObjectCreationAndLookup)
  • BenchmarkSetMarshalJSON (We already benchmark String() operations separately for Sets)

Note: This change is independent of the PR for improving the set union builtin (#4980), but very slightly improves performance there as well (around 5% or smaller improvement), likely from improved efficiency in Set construction.

srenatus
srenatus previously approved these changes Aug 15, 2022

@srenatus srenatus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM one nitpick 😄

Comment thread ast/term.go Outdated

@srenatus srenatus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@philipaconrad

Copy link
Copy Markdown
Member Author

Will update + squash-merge when green. 👍

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
@philipaconrad
philipaconrad merged commit 9d2b1ad into open-policy-agent:main Aug 15, 2022
@philipaconrad
philipaconrad deleted the set-insert-rework branch September 14, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants