Skip to content

Conversation

@akiradeveloper
Copy link
Owner

Cancelling possible read cache on only write-miss may be okay but it is very difficult for me to reason the correctness.

Always cancelling doesn't harm the performance so much because write-hit is not often and the lookup overhead is small enough.

For these reasons, I do this change.

@akiradeveloper akiradeveloper requested a review from Copilot July 10, 2025 04:23
@akiradeveloper akiradeveloper self-assigned this Jul 10, 2025
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 modifies the write path logic to always cancel read cache cells, regardless of whether a write operation results in a cache hit or miss. Previously, the might_cancel_read_cache_cell() function was only called when the cache lookup resulted in a miss (else clause), but now it executes unconditionally after cache lookup operations.

  • Removes conditional execution of read cache cell cancellation in write operations
  • Applies the change to both do_process_write and process_write_wa functions
  • Simplifies the logic by making read cache cancellation consistent across all write scenarios

Comment on lines 1181 to 1182

cache_lookup(wb, bio, &res);
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

[nitpick] There appears to be an unnecessary blank line removal after cache_lookup. This formatting change is unrelated to the core logic change and could be kept for better code readability.

Suggested change
cache_lookup(wb, bio, &res);
cache_lookup(wb, bio, &res);

Copilot uses AI. Check for mistakes.
Cancelling possible read cache on only write-miss may be okay
but it is very difficult for me to reason the correctness.

Always cancelling doesn't harm the performance so much because
write-hit is not often and the lookup overhead is small enough.

For these reasons, I do this change.

Signed-off-by: Akira Hayakawa <ruby.wktk@gmail.com>
@akiradeveloper akiradeveloper force-pushed the always-cancel-read-cache branch from f9b4b8a to eb17f8d Compare July 10, 2025 05:10
@akiradeveloper akiradeveloper merged commit b79b794 into master Jul 10, 2025
1 check passed
@akiradeveloper akiradeveloper deleted the always-cancel-read-cache branch July 10, 2025 05:24
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