reduce per-call allocations in CEL FieldGetters#795
Merged
Conversation
📝 WalkthroughWalkthroughUpdates Go module dependency for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
matthyx
previously approved these changes
Apr 27, 2026
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
Signed-off-by: Yakir Oren <yakiroren@gmail.com>
YakirOren
force-pushed
the
perf/cel-fieldgetter-allocs
branch
from
b7f406d to
87e7b54
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/utils/cel.go (1)
406-416:⚠️ Potential issue | 🟡 Minor
uidgetter is inconsistent — still returning the raw Go value.All other
UintTypefields in this map (attrSize, cmd, pid, ppid) returnceltypes.Uint(...), butuidstill returnsx.Raw.GetUid()unwrapped. This defeats the type-adaption savings the PR is targeting for this field.♻️ Proposed fix
"uid": { Type: celtypes.UintType, IsSet: isSet, GetFrom: ref.FieldGetter(func(target any) (any, error) { x := target.(*xcel.Object[CelEvent]) if x.Raw == nil { return nil, errCelObjectNil } - return x.Raw.GetUid(), nil + return celtypes.Uint(x.Raw.GetUid()), nil }), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/utils/cel.go` around lines 406 - 416, The uid field's getter returns the raw Go uint instead of a CEL uint value; inside the ref.FieldGetter for the "uid" entry (the function that accesses x := target.(*xcel.Object[CelEvent]) and calls x.Raw.GetUid()), wrap the returned value with celtypes.Uint(...) and return that (keeping the existing nil check for x.Raw and errCelObjectNil), so the getter returns a celtypes.Value like the other UintType fields (attrSize, cmd, pid, ppid).
🧹 Nitpick comments (1)
pkg/utils/cel.go (1)
522-551: Nit: error branch returns a raw""instead ofceltypes.String("").Minor consistency issue — every other return in this getter (and across the new HTTP getters) uses
celtypes.String(...), but L545 returns a bare"". The value is ignored on the error path so this is purely cosmetic, but worth tightening for consistency.♻️ Proposed tweak
if err != nil { - return "", err + return celtypes.String(""), err }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/utils/cel.go` around lines 522 - 551, The error branch in the ref.FieldGetter for the "body" field returns a raw "" string instead of a celtypes.String, causing inconsistency with other returns; update the error return within the getter (the branch that does io.ReadAll and checks err) to return celtypes.String("") along with err (i.e., replace the bare "" return with celtypes.String("")) so all successful/failed string returns use celtypes.String consistently; locate this in the anonymous GetFrom function inside the "body" Type definition in pkg/utils/cel.go (references: ref.FieldGetter, xcel.Object[CelEvent], errCelObjectNil, req.GetRequest()/GetBuf()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pkg/utils/cel.go`:
- Around line 406-416: The uid field's getter returns the raw Go uint instead of
a CEL uint value; inside the ref.FieldGetter for the "uid" entry (the function
that accesses x := target.(*xcel.Object[CelEvent]) and calls x.Raw.GetUid()),
wrap the returned value with celtypes.Uint(...) and return that (keeping the
existing nil check for x.Raw and errCelObjectNil), so the getter returns a
celtypes.Value like the other UintType fields (attrSize, cmd, pid, ppid).
---
Nitpick comments:
In `@pkg/utils/cel.go`:
- Around line 522-551: The error branch in the ref.FieldGetter for the "body"
field returns a raw "" string instead of a celtypes.String, causing
inconsistency with other returns; update the error return within the getter (the
branch that does io.ReadAll and checks err) to return celtypes.String("") along
with err (i.e., replace the bare "" return with celtypes.String("")) so all
successful/failed string returns use celtypes.String consistently; locate this
in the anonymous GetFrom function inside the "body" Type definition in
pkg/utils/cel.go (references: ref.FieldGetter, xcel.Object[CelEvent],
errCelObjectNil, req.GetRequest()/GetBuf()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe68c5f1-320c-4ba9-a620-41b15c43b53d
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
go.modpkg/utils/cel.go
matthyx
approved these changes
Apr 27, 2026
Performance Benchmark ResultsNode-Agent Resource Usage
Dedup Effectiveness (AFTER only)
Event Counters
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 by CodeRabbit
Chores
Performance