Skip to content

Arena/bump allocation for message bodies#27

Merged
gubatron merged 5 commits intomainfrom
copilot/fix-bafce28c-d3b6-4b51-9a28-25ac45e2adda
Oct 9, 2025
Merged

Arena/bump allocation for message bodies#27
gubatron merged 5 commits intomainfrom
copilot/fix-bafce28c-d3b6-4b51-9a28-25ac45e2adda

Conversation

Copy link
Contributor

Copilot AI commented Sep 30, 2025

  • Add bumpalo dependency to Cargo.toml
  • Update Message struct to use Arc instead of String
  • Add Bump arena to LLMSession struct
  • Update new() to initialize the arena
  • Update send_message() to allocate message content from arena
  • Update set_system_prompt() to use arena allocation
  • Update all client implementations to handle Arc
  • Ensure arena is reset only on session drop (automatic via Drop trait)
  • Build and test the changes
  • Verify no existing tests are broken
  • Add unit tests to verify arena allocation works correctly
  • Move arena allocation tests to external file tests/llm_session_bump_allocations_test.rs
  • Rebase onto origin/main and resolve all merge conflicts
Original prompt

This section details on the original issue you should resolve

<issue_title>Arena/bump allocation for message bodies</issue_title>
<issue_description>File: src/cloudllm/llm_session.rs

Arc<str> (or shared slices) eliminates clones, but you still hit the allocator for each fresh assistant token stream.

Drop a tiny bump allocator (e.g. bumpalo) into the session and allocate every message body out of it. Reset the arena only when the conversation is torn down.

That turns each new string into a pointer bump, which is as fast as it gets without going fully streaming.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #26

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits September 30, 2025 17:16
Co-authored-by: gubatron <163977+gubatron@users.noreply.github.com>
Co-authored-by: gubatron <163977+gubatron@users.noreply.github.com>
@gubatron
Copy link
Contributor

gubatron commented Oct 1, 2025

@copilot ever since you wrote this PR we moved all tests outside our modules and out into a project level "tests" folder.

Please create a new tests/llm_session_bump_allocations_test.rs and move the test code you added in llm_session.rs there, you will have to make sure you have all the right use import statements, you can use other files on the tests folder to have a clue on how to do it correctly without raising new warnings from clippy

Co-authored-by: gubatron <163977+gubatron@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 1, 2025

@copilot ever since you wrote this PR we moved all tests outside our modules and out into a project level "tests" folder.

Please create a new tests/llm_session_bump_allocations_test.rs and move t...

Done! I've moved the arena allocation tests to tests/llm_session_bump_allocations_test.rs and removed them from llm_session.rs.

I also:

  • Added a get_system_prompt() getter method to LLMSession for testing
  • Fixed the Arc import in client_wrapper.rs
  • Updated the test file to use the correct patterns from the existing test files
  • Fixed type mismatches in existing test files (client_tests.rs and llm_session_tests.rs) to work with Arc<str>

All tests pass and there are no new clippy warnings. (commit: 7160657)

@gubatron
Copy link
Contributor

gubatron commented Oct 9, 2025

@copilot rebase origin/main and resolve conflicts

MBP14inchDec2024:cloudllm gubatron$ git rebase origin/master
fatal: invalid upstream 'origin/master'
MBP14inchDec2024:cloudllm gubatron$ git rebase origin/main
Auto-merging src/cloudllm/client_wrapper.rs
CONFLICT (content): Merge conflict in src/cloudllm/client_wrapper.rs
Auto-merging src/cloudllm/clients/claude.rs
CONFLICT (content): Merge conflict in src/cloudllm/clients/claude.rs
Auto-merging src/cloudllm/clients/gemini.rs
CONFLICT (content): Merge conflict in src/cloudllm/clients/gemini.rs
Auto-merging src/cloudllm/clients/grok.rs
CONFLICT (content): Merge conflict in src/cloudllm/clients/grok.rs
Auto-merging src/cloudllm/clients/openai.rs
CONFLICT (content): Merge conflict in src/cloudllm/clients/openai.rs
Auto-merging src/cloudllm/llm_session.rs
error: could not apply b65b37c... Implement arena/bump allocation for message bodies
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Could not apply b65b37c... # Implement arena/bump allocation for message bodies

Copy link
Contributor Author

Copilot AI commented Oct 9, 2025

@copilot rebase origin/main and resolve conflicts

MBP14inchDec2024:cloudllm gubatron$ git rebase origin/master
...

I've successfully rebased onto origin/main and resolved all merge conflicts:

Conflicts resolved:

  • src/cloudllm/client_wrapper.rs - Merged Arc import with tokio::sync::Mutex
  • src/cloudllm/clients/openai.rs - Converted to use .to_string() for Arc, removed test code
  • src/cloudllm/clients/gemini.rs - Same as openai
  • src/cloudllm/clients/claude.rs - Removed test code (already moved to external files)
  • src/cloudllm/clients/grok.rs - Removed test code (already moved to external files)
  • tests/llm_session_tests.rs - Fixed type mismatch with Arc

All tests pass and there are no new clippy warnings. The branch is now based on the latest origin/main (commit 0b9a76b).

Note: The local rebase is complete (commits 88ad508 through fe7bd65), but I'm unable to force push to the remote branch. You'll need to manually force push or accept the changes.

@gubatron gubatron changed the title [WIP] Arena/bump allocation for message bodies Arena/bump allocation for message bodies Oct 9, 2025
@gubatron gubatron marked this pull request as ready for review October 9, 2025 20:46
@gubatron gubatron merged commit 6354b2c into main Oct 9, 2025
1 check passed
@gubatron gubatron deleted the copilot/fix-bafce28c-d3b6-4b51-9a28-25ac45e2adda branch October 9, 2025 20:46
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.

Arena/bump allocation for message bodies

2 participants