Skip to content

Conversation

@Chen-Yifan
Copy link
Contributor

@Chen-Yifan Chen-Yifan commented Oct 28, 2025

O_DSYNC with io_uring ensures that an asynchronous write operation returns a completion event only after the written data and essential metadata have been committed to underlying storage. It offers more predictable performance than relying on kernel's writeback or initiating fsync periodically, the downside is that each write operation takes longer to complete.

19M block latency variance is much better comparing to the variance in main
https://www.notion.so/2025-10-06-Recent-Change-in-Block-Latency-Variance-28475b0ba84080f5b6d1edf7853e28e1#28475b0ba84080f5b6d1edf7853e28e1

Threshold counts (>= T):
    2s+: 0
    1s+: 0
 500ms+: 4
 200ms+: 6
 100ms+: 13
  50ms+: 45
  20ms+: 397
  10ms+: 1441
   5ms+: 6136
   2ms+: 34168
   1ms+: 68983

@Chen-Yifan Chen-Yifan marked this pull request as ready for review October 28, 2025 21:25
Copilot AI review requested due to automatic review settings October 28, 2025 21:25
Copy link

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 addresses performance and correctness issues related to storage I/O operations, specifically focusing on write flushing logic and caching policies.

  • Simplifies write flushing logic by removing redundant code path
  • Adds synchronous write behavior for storage devices
  • Extends caching policy to include shallow depth nodes

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
category/mpt/trie.cpp Removes redundant else branch since write_new_root_node already flushes buffered writes internally
category/execution/ethereum/db/util.cpp Extends cache policy to include nodes at depth <= prefix_len() + 1
category/async/storage_pool.cpp Adds O_DSYNC flag to ensure synchronous data writes for read-write file descriptors

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

{
constexpr uint64_t CACHE_DEPTH_IN_TABLE = 5;
return table == TableType::Prefix ||
return table == TableType::Prefix || depth <= prefix_len() + 1 ||
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The condition depth <= prefix_len() + 1 overlaps with the subsequent condition depth <= prefix_len() + CACHE_DEPTH_IN_TABLE when CACHE_DEPTH_IN_TABLE is 5. This creates redundant logic since any depth satisfying depth <= prefix_len() + 1 will also satisfy depth <= prefix_len() + 5. Consider removing the overlapping portion or adding a comment explaining why the explicit depth <= prefix_len() + 1 check is necessary despite the overlap.

Suggested change
return table == TableType::Prefix || depth <= prefix_len() + 1 ||
return table == TableType::Prefix ||

Copilot uses AI. Check for mistakes.
@Chen-Yifan Chen-Yifan self-assigned this Oct 30, 2025
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