Skip to content

Speed up AutoHCC check in dtor#13998

Closed
pdillinger wants to merge 1 commit intofacebook:mainfrom
pdillinger:autohcc_fast_dtor_check
Closed

Speed up AutoHCC check in dtor#13998
pdillinger wants to merge 1 commit intofacebook:mainfrom
pdillinger:autohcc_fast_dtor_check

Conversation

@pdillinger
Copy link
Contributor

Summary: In #13964 I changed an expensive DEBUG check in ~AutoHyperClockTable to only run in ASAN builds. It's still expensive so I'm modifying it to scan only about one page beyond what we expect to have written to the anonymous mmap, rather than scanning the whole thing.

Test Plan: manually checked that lru_cache_test running time went from 5.0s to 4.0s after the change. Verified that existing unit test ClockCacheTest.Limits uses the full anonymous mmap to be sure it is sized as expected, by temporarily breaking AutoHyperClockTable::Grow() to allow slightly exceeding the anonymous mmap size.

Summary: In facebook#13964 I changed an expensive DEBUG check in
~AutoHyperClockTable to only run in ASAN builds. It's still expensive so
I'm modifying it to scan only about one page beyond what we expect to
have written to the anonymous mmap, rather than scanning the whole
thing.

Test Plan: manually checked that lru_cache_test running time went from
5.0s to 4.0s after the change. Verified that existing unit test
ClockCacheTest.Limits uses the full anonymous mmap to be sure it is
sized as expected, by temporarily breaking AutoHyperClockTable::Grow()
to allow slightly exceeding the anonymous mmap size.
@pdillinger pdillinger requested a review from cbi42 September 24, 2025 19:27
@meta-cla meta-cla bot added the CLA Signed label Sep 24, 2025
@facebook-github-bot
Copy link
Contributor

@pdillinger has imported this pull request. If you are a Meta employee, you can view this in D83178493.

Copy link
Contributor

@cbi42 cbi42 left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@facebook-github-bot
Copy link
Contributor

@pdillinger merged this pull request in 134cfb6.

cbi42 pushed a commit that referenced this pull request Sep 24, 2025
Summary:
In #13964 I changed an expensive DEBUG check in ~AutoHyperClockTable to only run in ASAN builds. It's still expensive so I'm modifying it to scan only about one page beyond what we expect to have written to the anonymous mmap, rather than scanning the whole thing.

Pull Request resolved: #13998

Test Plan: manually checked that lru_cache_test running time went from 5.0s to 4.0s after the change. Verified that existing unit test ClockCacheTest.Limits uses the full anonymous mmap to be sure it is sized as expected, by temporarily breaking AutoHyperClockTable::Grow() to allow slightly exceeding the anonymous mmap size.

Reviewed By: cbi42

Differential Revision: D83178493

Pulled By: pdillinger

fbshipit-source-id: a2bf093e98bf68b540c073800be7e193021f2692
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.

3 participants