chore(stdlib): Resolve bool and int constants during compilation#1803
chore(stdlib): Resolve bool and int constants during compilation#1803JakubOnderka wants to merge 2 commits into
Conversation
1bcee71 to
0537a0e
Compare
6b9db1b to
032bba2
Compare
fa620ed to
0219ff0
Compare
# Conflicts: # src/stdlib/util.rs
# Conflicts: # src/stdlib/util.rs
0219ff0 to
211f5a2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211f5a29c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Ok(match expr.resolve_constant(state) { | ||
| Some(cnst) => Self::Const(cnst.try_boolean()?), |
There was a problem hiding this comment.
Do not freeze loop-carried closure variables
When an optimized parameter is a variable that is constant at compile time but mutated inside an iterator closure, e.g. flag = true; for_each([1, 2]) -> |_, _| { contains("A", "a", case_sensitive: flag); flag = false }, this stores true in the compiled function and every iteration keeps using it. for_each closures preserve parent-scope mutations between iterations, so the second iteration should read the updated variable value; keep variable-backed expressions dynamic in repeated closures (the i64 helper below has the same issue).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Hi @JakubOnderka, this is great idea.
Can you run the following VRL program and compare the playground result with the result on your branch?
Summary
For simple method, resolving function parameters during runtime that are usually not defined or constants can take significant amount of time. This patch checks if these parameters are constants and if yes, resolve then in compilation phase. The advantage is also that variable type in that case is checked during compilation.
Benchmark results
compact
del
replace
log
starts_with
find
contains
contains_all
ends_with
split
parse_json
truncate
chunks
Change Type
Is this a breaking change?
How did you test this PR?
Does this PR include user facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here.