Skip to content

Conversation

@jstarks
Copy link
Member

@jstarks jstarks commented Nov 18, 2025

Performance traces show that growing and populating the vector of futures in disk_striped IO functions is expensive. Optimize this: use Iterator::collect to collect directly into the JoinAll that we'll await, using an iterator that implements TrustedLen so that the vector can allocate its buffer once and construct the futures in place.

Also, clean up some impossible failure paths.

In the `disk_striped` IO paths, use `Iterator::collect` instead of
`push`ing into a `Vec`. Reimplement the chunk iterator to ensure it
implements `TrustedLen`. This should ensure there's exactly one
allocation for the futures array per IO and that the compiler constructs
the futures directly into the array.
Copilot AI review requested due to automatic review settings November 18, 2025 05:08
@jstarks jstarks requested review from a team as code owners November 18, 2025 05:08
Copilot finished reviewing on behalf of jstarks November 18, 2025 05:11
Copy link
Contributor

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 the performance of futures vector creation in disk_striped I/O functions by using Iterator::collect with iterators that implement TrustedLen, enabling single-allocation vector construction.

Key changes:

  • Replaced custom ChunkIter struct with inline Range::map iterator for TrustedLen optimization
  • Simplified error handling by replacing verbose IoError enum with simpler LowerError struct
  • Refactored futures creation from for loops with Vec::push to iterator chains with .collect()
  • Improved test assertions using idiomatic .expect() and .expect_err() methods

@jstarks jstarks requested a review from a team as a code owner November 18, 2025 05:52
@github-actions
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants