Skip to content

feat: Optimize CommitDiff using efficient merge_diff_copy that avoids heap allocation #113

Merged
snawaz merged 1 commit into
mainfrom
snawaz/optimize-commit-diff
Oct 31, 2025
Merged

feat: Optimize CommitDiff using efficient merge_diff_copy that avoids heap allocation #113
snawaz merged 1 commit into
mainfrom
snawaz/optimize-commit-diff

Conversation

@snawaz
Copy link
Copy Markdown
Collaborator

@snawaz snawaz commented Oct 28, 2025

It is the third PR in the current stack.

The previous PR #110 used an inefficient approach to diff application that allocates memory on the heap, which can be problematic for larger accounts.

Solution

This PR adds a new merge_diff_copy() function that applies diffs directly to a destination buffer without requiring additional memory allocation. This function takes a destination buffer, original data, and a DiffSet.. and then merges the original with the diff to produce the updated version in the destination buffer. The whole approach avoids unnecessary memory allocation.

Apart from that, it also adds enum NewState to represent either full bytes or a diff, and process_commit_state_internal() is modified to handle both full data and diffs using NewState.

Summary by CodeRabbit

  • Refactor

    • Improved internal diff application and state management for enhanced performance and clarity.
  • Bug Fixes

    • Enhanced error handling with new validation for diff operations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 28, 2025

Walkthrough

This PR refactors diff application in the commitment processor by introducing a NewState enum that abstracts between full byte representations and diff-based state, alongside a new merge_diff_copy utility function to apply diffs to a destination buffer. Error handling is extended with a new MergeDiffError variant.

Changes

Cohort / File(s) Summary
Diff module extension
src/diff/algorithm.rs, src/error.rs
Adds public merge_diff_copy() function to apply diff segments to a destination buffer; exports OffsetInData type; adds MergeDiffError variant (discriminant 17) for precondition violations; updates InvalidDiff error message to specify "Invalid length of diff".
State abstraction layer
src/processor/fast/commit_state.rs
Introduces NewState<'a> enum with FullBytes(&'a [u8]) and Diff(DiffSet<'a>) variants; updates CommitStateInternalArgs.commit_state_bytes field to use NewState<'a> instead of &[u8]; adds data_len() method to compute length polymorphically; modifies commit state copy logic to handle both variants (full bytes and diff merge).
State abstraction usage
src/processor/fast/commit_diff.rs, src/processor/fast/commit_state_from_buffer.rs
Replaces direct diff application with NewState::Diff(diffset) in commit_diff; removes apply_diff_copy invocation; updates commit_state_from_buffer to wrap state buffer as NewState::FullBytes(&state) when constructing CommitStateInternalArgs.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant CommitState as commit_state.rs
    participant DiffModule as diff/algorithm.rs
    participant Account as Delegated Account

    Caller->>CommitState: process_commit_state_internal(args: NewState)
    alt args is FullBytes
        CommitState->>CommitState: copy FullBytes to destination
    else args is Diff
        CommitState->>Account: read original account data
        Account-->>CommitState: original bytes
        CommitState->>DiffModule: merge_diff_copy(destination, original, diffset)
        DiffModule->>DiffModule: apply diff segments to destination
        DiffModule-->>CommitState: Result
    end
    CommitState-->>Caller: commit state applied
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • commit_state.rs: Core change with new NewState enum and polymorphic handling; verify data_len() consistency and both variant branches (FullBytes copy vs. Diff merge) are correct.
  • commit_diff.rs: Verify transition from inline apply_diff_copy to NewState::Diff abstraction and CommitStateInternalArgs field compatibility.
  • algorithm.rs: Review new merge_diff_copy function logic for boundary correctness and error validation.
  • Error variant changes: Confirm error message updates and new discriminant do not conflict with existing enum values.

Possibly related PRs

Suggested reviewers

  • thlorenz
  • GabrielePicco

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is incomplete and does not follow the required template structure. The description is missing several critical sections including the Status/Type/Issue table, a formal Problem section, Before & After screenshots, and Deploy Notes. While the description does provide relevant context about the PR being part of a stack and includes a Solution section with technical details about merge_diff_copy and the NewState enum, it fails to meet the structured requirements of the template. The absence of a formal Problem statement and status information means reviewers lack key context about the PR's purpose and scope, and there is no guidance on deployment considerations. The author should revise the PR description to include all required template sections: add a Status/Type/Issue table at the top with appropriate selections, formally state the Problem being solved (heap allocation inefficiency in diff application), include Before & After code examples or explanations if applicable, document any Deploy Notes regarding new dependencies or breaking changes, and expand the Other changes section if needed. This will ensure the PR is properly documented for review and future reference.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "feat: Optimize CommitDiff using efficient merge_diff_copy that avoids heap allocation" directly and accurately summarizes the main objective of the changeset. It clearly identifies the primary change (introducing an efficient merge_diff_copy function), specifies the component being optimized (CommitDiff), and articulates the key benefit (avoiding heap allocation). The title is concise, specific, and avoids vague language, making it immediately clear to reviewers scanning the commit history what this PR accomplishes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch snawaz/optimize-commit-diff

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator Author

snawaz commented Oct 28, 2025

@snawaz snawaz changed the title feat: Optimize commit diff feat: Optimize commit diff using efficient merge_diff_copy that avoids heap allocation Oct 28, 2025
@snawaz snawaz changed the title feat: Optimize commit diff using efficient merge_diff_copy that avoids heap allocation feat: Optimize CommitDiff using efficient merge_diff_copy that avoids heap allocation Oct 28, 2025
@snawaz snawaz force-pushed the snawaz/commit-diff branch from 0f5b8cb to b32a0f7 Compare October 28, 2025 13:41
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 4 times, most recently from 534505a to 78e483c Compare October 28, 2025 16:05
@snawaz snawaz force-pushed the snawaz/commit-diff branch from b32a0f7 to b39ff4e Compare October 28, 2025 16:05
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch from 78e483c to 98e5f41 Compare October 29, 2025 07:28
@snawaz snawaz force-pushed the snawaz/commit-diff branch 2 times, most recently from d639a5c to 04fe332 Compare October 29, 2025 07:43
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch from 98e5f41 to a09685f Compare October 29, 2025 07:43
@snawaz snawaz force-pushed the snawaz/commit-diff branch from 04fe332 to 1db562c Compare October 29, 2025 07:55
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 3 times, most recently from 01758bc to 02c6224 Compare October 29, 2025 09:18
@snawaz snawaz force-pushed the snawaz/commit-diff branch from cb49fb2 to 5d1c8e6 Compare October 29, 2025 10:02
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch from 02c6224 to c37ba3e Compare October 29, 2025 10:02
@snawaz snawaz force-pushed the snawaz/commit-diff branch from 5d1c8e6 to ffefbfd Compare October 29, 2025 10:03
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 2 times, most recently from 731e318 to b33f9f1 Compare October 29, 2025 10:16
@snawaz snawaz force-pushed the snawaz/commit-diff branch from ffefbfd to 35d4201 Compare October 29, 2025 10:16
@snawaz snawaz requested a review from GabrielePicco October 29, 2025 12:02
@snawaz snawaz marked this pull request as ready for review October 29, 2025 12:03
@snawaz snawaz force-pushed the snawaz/commit-diff branch from 35d4201 to fe0e096 Compare October 29, 2025 12:45
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch from b33f9f1 to 2fed9a7 Compare October 29, 2025 12:46
@snawaz snawaz changed the title feat: Optimize CommitDiff using efficient merge_diff_copy that avoids heap allocation feat: Optimize CommitDiff using efficient merge_diff_copy that avoids heap allocation Oct 30, 2025
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 2 times, most recently from e29cca1 to 4a5156b Compare October 30, 2025 10:07
@snawaz snawaz force-pushed the snawaz/commit-diff branch from 9728017 to 6c9fc44 Compare October 30, 2025 17:59
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 2 times, most recently from 57a4834 to a4a5218 Compare October 30, 2025 18:19
@snawaz snawaz force-pushed the snawaz/commit-diff branch 2 times, most recently from b4992ce to 0af4ec2 Compare October 30, 2025 18:42
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 3 times, most recently from 6a14dcc to 97ac612 Compare October 30, 2025 19:33
Copy link
Copy Markdown
Contributor

@GabrielePicco GabrielePicco left a comment

Choose a reason for hiding this comment

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

LGTM!

@snawaz snawaz force-pushed the snawaz/commit-diff branch from 0af4ec2 to b0922a1 Compare October 31, 2025 06:18
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 2 times, most recently from 6a85e7f to 59aca02 Compare October 31, 2025 14:51
@snawaz snawaz force-pushed the snawaz/commit-diff branch from b0922a1 to 3099cc4 Compare October 31, 2025 14:51
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch 2 times, most recently from 3a4b154 to a92c9e3 Compare October 31, 2025 15:16
@snawaz snawaz force-pushed the snawaz/commit-diff branch from a76b7db to 3705937 Compare October 31, 2025 15:16
@snawaz snawaz force-pushed the snawaz/optimize-commit-diff branch from a92c9e3 to 3836029 Compare October 31, 2025 15:24
Base automatically changed from snawaz/commit-diff to main October 31, 2025 15:34
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 97174ec and 383919b.

📒 Files selected for processing (5)
  • src/diff/algorithm.rs (5 hunks)
  • src/error.rs (1 hunks)
  • src/processor/fast/commit_diff.rs (2 hunks)
  • src/processor/fast/commit_state.rs (5 hunks)
  • src/processor/fast/commit_state_from_buffer.rs (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-15T11:45:25.802Z
Learnt from: snawaz
Repo: magicblock-labs/delegation-program PR: 107
File: src/entrypoint.rs:141-144
Timestamp: 2025-10-15T11:45:25.802Z
Learning: In the delegation-program codebase, prefer using `log!` (from pinocchio_log) over `msg!` for error and panic scenarios in the entrypoint code, as per maintainer preference.

Applied to files:

  • src/processor/fast/commit_state.rs
🧬 Code graph analysis (1)
src/processor/fast/commit_state.rs (1)
src/diff/algorithm.rs (1)
  • merge_diff_copy (240-262)

Comment thread src/diff/algorithm.rs
@snawaz snawaz merged commit e8d0393 into main Oct 31, 2025
5 checks passed
@snawaz snawaz deleted the snawaz/optimize-commit-diff branch October 31, 2025 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants