Avoid Vec+join in jq Replace/Append (DSK-137)#178
Conversation
Replace collect/join in Replace and Append modes with a single-pass write into the destination string. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant jq_evaluate
participant format_outputs_joined
jq_evaluate->>format_outputs_joined: outputs (&[Val])
format_outputs_joined-->>jq_evaluate: 結合済み String
jq_evaluate->>jq_evaluate: ev.message へ設定 (Replace/Append)
重大度: 低。振る舞いは変更前と等価（同じ結合ロジックの抽出のみ）で、非同期処理・K8s・セキュリティへの影響はなし。 Possibly related issues
🚥 Pre-merge checks | ❌ 2❌ Failed checks (2 warnings)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/rustern-core/src/pipeline/jq_evaluate.rs`:
- Around line 27-36: The hot-path formatter in format_outputs_joined currently
starts with String::new(), which can trigger repeated reallocations as outputs
grow. Update this helper to preallocate the buffer with an appropriate capacity
hint using with_capacity, based on the expected number of Val items and
separators, while keeping the existing join logic in place. Focus the change
inside format_outputs_joined so the per-line allocation behavior in jq_evaluate
is reduced without altering the rendered output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 180864f5-d536-49ff-ac86-7c84af5c7a75
📒 Files selected for processing (1)
crates/rustern-core/src/pipeline/jq_evaluate.rs
| fn format_outputs_joined(outputs: &[Val]) -> String { | ||
| let mut rendered = String::new(); | ||
| for (i, v) in outputs.iter().enumerate() { | ||
| if i > 0 { | ||
| rendered.push('\n'); | ||
| } | ||
| let _ = write!(rendered, "{v}"); | ||
| } | ||
| rendered | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
軽微: 容量事前確保でホットパスの再割当てを削減可能
String::new() は初期容量ゼロのため、要素数が多い場合に複数回の再割当てが発生し得ます。パイプラインのホットパスにおける per-line allocation 最適化が本PRの目的なので、with_capacity によるヒントもついでに検討すると良さそうです。
As per path instructions, "Performance on hot paths (per-line allocations, regex reuse)" が pipeline ディレクトリの重点項目です。
♻️ 提案
fn format_outputs_joined(outputs: &[Val]) -> String {
- let mut rendered = String::new();
+ let mut rendered = String::with_capacity(outputs.len() * 16);
for (i, v) in outputs.iter().enumerate() {
if i > 0 {
rendered.push('\n');
}
let _ = write!(rendered, "{v}");
}
rendered
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn format_outputs_joined(outputs: &[Val]) -> String { | |
| let mut rendered = String::new(); | |
| for (i, v) in outputs.iter().enumerate() { | |
| if i > 0 { | |
| rendered.push('\n'); | |
| } | |
| let _ = write!(rendered, "{v}"); | |
| } | |
| rendered | |
| } | |
| fn format_outputs_joined(outputs: &[Val]) -> String { | |
| let mut rendered = String::with_capacity(outputs.len() * 16); | |
| for (i, v) in outputs.iter().enumerate() { | |
| if i > 0 { | |
| rendered.push('\n'); | |
| } | |
| let _ = write!(rendered, "{v}"); | |
| } | |
| rendered | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/rustern-core/src/pipeline/jq_evaluate.rs` around lines 27 - 36, The
hot-path formatter in format_outputs_joined currently starts with String::new(),
which can trigger repeated reallocations as outputs grow. Update this helper to
preallocate the buffer with an appropriate capacity hint using with_capacity,
based on the expected number of Val items and separators, while keeping the
existing join logic in place. Focus the change inside format_outputs_joined so
the per-line allocation behavior in jq_evaluate is reduced without altering the
rendered output.
Source: Path instructions
1 participant
Summary
format_outputs_joinedto write jq filter outputs into oneStringwith newline separators.collect::<Vec<_>>().join("\n").Test plan
cargo test -p rustern-core --lib pipeline::jq_evaluateMade with Cursor
Summary by CodeRabbit