Skip to content

Perform less last write time I/O checks during indexing#703

Merged
jviotti merged 1 commit intomainfrom
reduce-last-write-time
Mar 4, 2026
Merged

Perform less last write time I/O checks during indexing#703
jviotti merged 1 commit intomainfrom
reduce-last-write-time

Conversation

@jviotti
Copy link
Member

@jviotti jviotti commented Mar 4, 2026

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@augmentcode
Copy link

augmentcode bot commented Mar 4, 2026

🤖 Augment PR Summary

Summary: Reduces filesystem I/O during indexing by caching last_write_time results inside BuildAdapterFilesystem::marks.
Changes: mark() now stores the computed mtime in the in-memory cache to avoid repeated std::filesystem::last_write_time calls within the same run.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

// change. If we did build it, refreshing already set a synthetic timestamp
// that the cache lookup above would have returned instead
std::unique_lock lock{this->mutex};
this->marks.emplace(path, value);
Copy link

Choose a reason for hiding this comment

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

There’s a potential race here: another thread could call refresh(path) after the initial cache lookup but before this emplace, causing emplace to fail while mark() still returns the older value (so the returned mark can differ from what’s cached). Consider ensuring mark() returns the value actually stored in marks under the write lock so callers see a consistent mark.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark Index (community)

Details
Benchmark suite Current: 67eabad Previous: f68e4e2 Ratio
Add one schema (0 existing) 41 ms 40 ms 1.02
Add one schema (100 existing) 418 ms 382 ms 1.09
Add one schema (1000 existing) 4312 ms 3882 ms 1.11

This comment was automatically generated by workflow using github-action-benchmark.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Benchmark Index (enterprise)

Details
Benchmark suite Current: 67eabad Previous: f68e4e2 Ratio
Add one schema (0 existing) 49 ms 43 ms 1.14
Add one schema (100 existing) 404 ms 422 ms 0.96
Add one schema (1000 existing) 4059 ms 4071 ms 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@jviotti jviotti merged commit 2899731 into main Mar 4, 2026
6 checks passed
@jviotti jviotti deleted the reduce-last-write-time branch March 4, 2026 16:28
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.

1 participant