performance(runtime)!: Copy-on-Write value optimization #1349
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
Zettroke
changed the title
10 tasks
pront
changed the title
|
Hi @Zettroke, thank you for submitting this. I added it to my list for review. Is this a breaking change for users or for developers only? |
|
Hi @pront, no it's a breaking change only in terms of crate API. VRL itself is unchanged. |
bruceg
reviewed
Mar 26, 2025
src/compiler/expression/array.rs
Outdated
| @@ -12,12 +12,26 @@ use crate::{ | |||
|
|
|||
| #[derive(Debug, Clone, PartialEq)] | |||
| pub struct Array { | |||
| resolved: Option<Value>, | |||
There was a problem hiding this comment.
How hard would it be to separate out the changes to cache the resolved value of these array and object expressions? On first reading, I thought this was part of the Value CoW optimization and then realized it was doing something different.
There was a problem hiding this comment.
Sure. I just wanted to show bigger gains ;)
bruceg
reviewed
Mar 26, 2025
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.
Summary
This request introduces CoW logic into vrl
Value. It aims to allow passingValuebetween function by reference without the need for copies. It also makes usingresolve_constantto precompute expressions viable, just makingObjectandArrayexpressions use it yielded 15% more throughput.This request requires vectordotdev/vector#22693 to work in vector.
Change Type
Implementation
All of it is based on
Arc::make_mutAPI. To reduce the number of code changes I decided to use customDerefMutwhich calls toArc::make_mut. While it can potentially lead to excessive copies it's still strictly better than before. But I will understand if you want a more explicit API for this.I decided to wrap it up with Arc-only
ObjectandArrayvariants. This avoids indirection for trivially copyable scalar values, andBytesare already ref-counted. Also, these variants already include an indirection, so overhead is not that noticeable.Benchmarks
I used our set of 120+ VRL transforms used for event normalization.
On average this change yielded almost 2x speedup in pure vrl benchmarks.
I attached the full table with results, sadly I'm not allowed to provide sources of our VRL transforms, so I kindly ask you to benchmark on your rule-sets to see speedups in other use cases
Benchmark results
Google sheets
Is this a breaking change
How did you test this PR?
We used included unit tests + we already used this change in our vector load-testing setup.
Does this PR include user facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here.