Skip to content

Conversation

roberto-bayardo
Copy link
Collaborator

@roberto-bayardo roberto-bayardo commented Jul 23, 2025

Improves performance of most archive::immutable benchmarks by ~30%. This code uses journal.get instead of get_exact. Since prunable archive currently uses get_exact, there is no performance impact on that.

@roberto-bayardo roberto-bayardo marked this pull request as ready for review July 23, 2025 20:08
@roberto-bayardo roberto-bayardo requested a review from Copilot July 23, 2025 20:08
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes blob reading performance by reducing the number of read_at syscalls from 3 to 2 when reading journal entries. Instead of making separate calls to read the item data and checksum, the code now reads both in a single operation and extracts the components from the combined buffer.

  • Combines item and checksum reads into a single read_at call with a larger buffer
  • Extracts item data and checksum from the combined buffer using slicing
  • Removes unnecessary .as_ref() calls where direct references are available

@roberto-bayardo roberto-bayardo force-pushed the faster-variable-log-read branch 5 times, most recently from b888130 to 663a599 Compare July 23, 2025 21:00
@roberto-bayardo roberto-bayardo changed the title [storage/log/variable] reduce # of blob read_at calls from 3 to 2 in read [storage/log + storage/archive] reduce # of blob read_at calls from 3 to 2 in read Jul 23, 2025
@roberto-bayardo roberto-bayardo force-pushed the faster-variable-log-read branch from b29ef74 to 663a599 Compare July 23, 2025 21:14
@roberto-bayardo roberto-bayardo changed the title [storage/log + storage/archive] reduce # of blob read_at calls from 3 to 2 in read [storage/log/variable] reduce # of blob read_at calls from 3 to 2 in read and read_buffered Jul 23, 2025
Copy link
Contributor

@patrick-ogrady patrick-ogrady left a comment

Choose a reason for hiding this comment

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

small nits!

Copy link
Contributor

@patrick-ogrady patrick-ogrady left a comment

Choose a reason for hiding this comment

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

🚀

@patrick-ogrady patrick-ogrady merged commit a3a186f into main Jul 23, 2025
33 checks passed
@patrick-ogrady patrick-ogrady deleted the faster-variable-log-read branch July 23, 2025 21:49
Copy link

codecov bot commented Jul 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.20%. Comparing base (1ba5c93) to head (1a415d7).
Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main    #1313      +/-   ##
==========================================
- Coverage   91.20%   91.20%   -0.01%     
==========================================
  Files         248      248              
  Lines       62084    62083       -1     
==========================================
- Hits        56623    56622       -1     
  Misses       5461     5461              
Files with missing lines Coverage Δ
storage/src/journal/variable.rs 92.91% <100.00%> (-0.01%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ba5c93...1a415d7. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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