apollo_storage: add mmap file helper methods

[dean-starkware]

This PR adds two helper methods in apollo_storage’s mmap implementation- a small refactor / safety improvement extracted as a prerequisite for the upcoming storage batching PR.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2026-01-05T09:27:10Z
• Started at 2026-01-05T09:31:07Z
• Started at 2026-01-05T11:09:14Z
• Started at 2026-01-05T11:13:28Z
• Started at 2026-01-05T11:19:04Z
• Started at 2026-01-05T11:23:05Z
• Started at 2026-01-06T08:32:03Z
• Started at 2026-01-07T11:42:48Z
• Started at 2026-01-07T13:55:06Z
• Started at 2026-01-08T14:07:56Z
• Started at 2026-01-09T12:16:53Z

[matanl-starkware]

@matanl-starkware reviewed 1 file and all commit messages, and made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @dean-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 148 at r2 (raw file):

}

impl<V: ValueSerde> MMapFile<V> {

IIUC, all MMapFile functions should be called under lock.
If that's correct, lets document this.

Code quote:

MMapFile

---

crates/apollo_storage/src/mmap_file/mod.rs line 163 at r2 (raw file):

        debug!(
            "Growing file to size: {} (target: {}, steps: {})",
            new_size, target_size, growth_steps

IMO number of steps doesn't really matter here. Consider logging the growth_size instead (i.e. step * steps)

Code quote:

growth_steps

---

crates/apollo_storage/src/mmap_file/mod.rs line 191 at r2 (raw file):

        let mmap_slice = &mut self.mmap[offset..final_offset];
        assert!(
            mmap_slice.len() >= len,

With current implementation, they must be equal

Suggestion:

==

---

crates/apollo_storage/src/mmap_file/mod.rs line 253 at r2 (raw file):

impl<V: ValueSerde> FileHandler<V, RW> {
    fn grow_file_if_needed(&mut self, offset: usize) {

I don't understand why we need this function now.

Code quote (i):

grow_file_if_needed

Code snippet (ii):

grow_to_target(offset + mmap_file.config.max_object_size)

---

crates/apollo_storage/src/mmap_file/mod.rs line 241 at r2 (raw file):

                .mmap
                .flush_async_range(offset, len)
                .expect("Failed to asynchronously flush the mmap after inserting");

Seems that this part was removed.
Why?

Code quote:

            mmap_file
                .mmap
                .flush_async_range(offset, len)
                .expect("Failed to asynchronously flush the mmap after inserting");

---

crates/apollo_storage/src/mmap_file/mod.rs line 157 at r1 (raw file):

        self.flush();
        // Calculate how many growth steps needed, rounding up.
        let growth_needed = target_size.saturating_sub(self.size);

At this stage, we know for sure that target > actual.
Consider a simple sub:

Suggestion:

target_size - self.size

[dean-starkware]

@dean-starkware made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @matanl-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 157 at r1 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

At this stage, we know for sure that target > actual.
Consider a simple sub:

Done.

---

crates/apollo_storage/src/mmap_file/mod.rs line 148 at r2 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

IIUC, all MMapFile functions should be called under lock.
If that's correct, lets document this.

Done.

---

crates/apollo_storage/src/mmap_file/mod.rs line 163 at r2 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

IMO number of steps doesn't really matter here. Consider logging the growth_size instead (i.e. step * steps)

Done.

---

crates/apollo_storage/src/mmap_file/mod.rs line 191 at r2 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

With current implementation, they must be equal

Done.

---

crates/apollo_storage/src/mmap_file/mod.rs line 241 at r2 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

Seems that this part was removed.
Why?

You're right- it was removed during refactoring but shouldn't have been. The flush_async_range starts persisting data in the background without blocking, so it improves durability at no performance cost. I'll add it back.

---

crates/apollo_storage/src/mmap_file/mod.rs line 253 at r2 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

I don't understand why we need this function now.

We need grow_file_if_needed because it handles pre-allocation for the next write, while grow_to_target (called inside write_at_current_offset) only grows the file to fit the current write. Without grow_file_if_needed, we'd grow the file on almost every append() call, since we'd only have exactly enough space for what we just wrote. With it, after each write we ensure there's room for at least one more max-sized object, so subsequent writes often don't need to trigger a grow. I tried removing it and the grow_file test fails because it validates this pre-allocation behavior- the test expects the file to be pre-grown to accommodate future writes, not just the current one.

[matanl-starkware]

@matanl-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 5 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dean-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 253 at r2 (raw file):

Previously, dean-starkware wrote…

We need grow_file_if_needed because it handles pre-allocation for the next write, while grow_to_target (called inside write_at_current_offset) only grows the file to fit the current write. Without grow_file_if_needed, we'd grow the file on almost every append() call, since we'd only have exactly enough space for what we just wrote. With it, after each write we ensure there's room for at least one more max-sized object, so subsequent writes often don't need to trigger a grow. I tried removing it and the grow_file test fails because it validates this pre-allocation behavior- the test expects the file to be pre-grown to accommodate future writes, not just the current one.

I still don't get it.

1. The grow_to_target has an internal check that if the current size is sufficient, nothing is done (first check in the function).
2. If growth is required, at least onegrowth_step will be added to the size. So sequential small writes shouldn't trigger actual growth each time.
3. If the tests fail - let's understand why... Maybe the behavioral change requires some modifications...

[dean-starkware]

@dean-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matanl-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 253 at r2 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

I still don't get it.

1. The grow_to_target has an internal check that if the current size is sufficient, nothing is done (first check in the function).
2. If growth is required, at least onegrowth_step will be added to the size. So sequential small writes shouldn't trigger actual growth each time.
3. If the tests fail - let's understand why... Maybe the behavioral change requires some modifications...

You're right that grow_to_target grows in growth_step increments, but that's not the issue. The problem is when the growth check happens.

Without grow_file_if_needed- We only grow to fit the current write (offset + current_object_size). If the current object is small but the next object is large (up to max_object_size), we'll need to grow again on the very next write.

With grow_file_if_needed- After each write, we ensure the file can accommodate offset + max_object_size, guaranteeing space for at least one more max-sized object. This pre-allocation means the next write (of any size ≤ max_object_size) won't trigger a growth.

The test validates this pre-allocation behavior- after the first two appends, the file is already grown to accommodate the third append without additional growth. Without grow_file_if_needed, each append would only grow to fit itself, not prepare for the next one, causing the test expectations to fail.

So- grow_file_if_needed implements write-ahead space reservation, ensuring we always have room for the next write, not just the current one. This is especially important for variable-sized objects where we can't predict the next write's size.

[dean-starkware]

@dean-starkware made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matanl-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 253 at r2 (raw file):

Previously, dean-starkware wrote…

You're right that grow_to_target grows in growth_step increments, but that's not the issue. The problem is when the growth check happens.

Without grow_file_if_needed- We only grow to fit the current write (offset + current_object_size). If the current object is small but the next object is large (up to max_object_size), we'll need to grow again on the very next write.

With grow_file_if_needed- After each write, we ensure the file can accommodate offset + max_object_size, guaranteeing space for at least one more max-sized object. This pre-allocation means the next write (of any size ≤ max_object_size) won't trigger a growth.

The test validates this pre-allocation behavior- after the first two appends, the file is already grown to accommodate the third append without additional growth. Without grow_file_if_needed, each append would only grow to fit itself, not prepare for the next one, causing the test expectations to fail.

So- grow_file_if_needed implements write-ahead space reservation, ensuring we always have room for the next write, not just the current one. This is especially important for variable-sized objects where we can't predict the next write's size.

Update: the function was deleted. You were absolutely right.

[matanl-starkware]

@matanl-starkware reviewed 2 files and all commit messages, made 2 comments, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dean-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 194 at r4 (raw file):

        _mode: PhantomData,
    };
    write_file_handler.grow_file_if_needed(0);

Consider adding new() function to MMapFile, which also calls grow_to_target right away so we would not hit the grow on the first write.

Code quote:

    write_file_handler.grow_file_if_needed(0);

---

crates/apollo_storage/src/mmap_file/mod.rs line 246 at r4 (raw file):

        }
        let location = LocationInFile { offset, len };
        self.grow_file_if_needed(location.next_offset());

In the new implementation, growth occurs before the write is performed.
Previously, it was done after the write.
Do you think that matters?

Code quote:

self.grow_file_if_needed(location.next_offset());

[dean-starkware]

@dean-starkware made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @matanl-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 194 at r4 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

Consider adding new() function to MMapFile, which also calls grow_to_target right away so we would not hit the grow on the first write.

Done.

---

crates/apollo_storage/src/mmap_file/mod.rs line 246 at r4 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

In the new implementation, growth occurs before the write is performed.
Previously, it was done after the write.
Do you think that matters?

I think it's better now- both approaches are safe and result in similar file sizes, but it makes more sense to do it before rather after, in my opinion.

[matanl-starkware]

@matanl-starkware reviewed 2 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dean-starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 299 at r5 (raw file):

        locations
    }
}

This part belongs to the following PR

Code quote:

impl<V: ValueSerde> FileHandler<V, RW> {
    /// Appends multiple values in a single lock acquisition and returns their locations.
    pub(crate) fn append_batch(&mut self, vals: &[&V::Value]) -> Vec<LocationInFile> {
        if vals.is_empty() {
            return Vec::new();
        }

        let serialized_vals: Vec<Vec<u8>> = vals
            .iter()
            .map(|val| V::serialize(val).expect("Should be able to serialize"))
            .collect();
        let total_len: usize = serialized_vals.iter().map(|v| v.len()).sum();

        let mut locations = Vec::with_capacity(vals.len());
        let mut mmap_file = self.mmap_file.lock().expect("Lock should not be poisoned");
        let start_offset = mmap_file.offset;
        mmap_file.grow_to_target(start_offset + total_len);

        for serialized in &serialized_vals {
            let location = mmap_file.write_at_current_offset(serialized);
            locations.push(location);
        }

        locations
    }
}

[matanl-starkware]

@matanl-starkware reviewed 1 file and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dean-starkware and @Itay-Tsabary-Starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 222 at r6 (raw file):

            len,
            mmap_slice.len()
        );

I think we can remove this assert now.
Previously, it verified that mmap had enough space to write into.
But currently it only verifies that A+B=C.

Leaving this comment as a blocker, since @Itay-Tsabary-Starkware also asked to review this PR.

Code quote:

        assert!(
            mmap_slice.len() == len,
            "Mmap slice length mismatch: expected={}, actual={}",
            len,
            mmap_slice.len()
        );

[dean-starkware]

@dean-starkware made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Itay-Tsabary-Starkware).

---

crates/apollo_storage/src/mmap_file/mod.rs line 299 at r5 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

This part belongs to the following PR

 I actually already caught and fixed this earlier.
The append_batch method has been removed from this mmap PR- it now only exists in the batching PR where it's actually used.
I believe your comment was from before the fix was pushed. The current state should be what you're asking for. Let me know if you still see any issues.

---

crates/apollo_storage/src/mmap_file/mod.rs line 222 at r6 (raw file):

Previously, matanl-starkware (Matan Lior) wrote…

I think we can remove this assert now.
Previously, it verified that mmap had enough space to write into.
But currently it only verifies that A+B=C.

Leaving this comment as a blocker, since @Itay-Tsabary-Starkware also asked to review this PR.

Agree- fixed it.

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 1 comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @matanl-starkware).

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware reviewed 2 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matanl-starkware).

[matanl-starkware]

@matanl-starkware reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dean-starkware).