[SharedCache] Cache type libraries #6196
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The existing view-specific state was stored in several global unordered maps. Many of these were accessed without locking, including `viewSpecificMutexes`, which is racy in the face of multiple threads. View-specific state is stored in a new heap-allocated `ViewSpecificState` struct that is reference counted via `std::shared_ptr`. A static map holds a `std::weak_ptr` to each view-specific state, keyed by session id. `SharedCache` retrieves its view-specific state during its constructor. Since `ViewSpecificState` is reference counted it will naturally be deallocated when the last `SharedCache` instance that references it goes away. Its corresponding entry will remain in the static map, though since it only holds a `std::weak_ptr` rather than any state it will not use much memory. The next time view-specific state is retrieved any expired entries will be removed from the map.
They're surprisingly expensive to look up.
This was referenced Nov 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type libraries are surprisingly expensive to look up via
BinaryView. Caching them can speed upFindSymbolAtAddrAndApplyToAddrsignificantly.View-specific state
The first commit refactors how view-specific state is stored to make it easier to work with. This was motivated by wanting to be able to clean up view-specific state when a view goes away, and some data races I noticed in how the existing view-specific state is accessed.
The existing view-specific state was stored in several global unordered maps. Many of these were accessed without locking, including
viewSpecificMutexes, which is racy in the face of multiple threads. The view-specific state is never cleaned up and remains in place after the given view is gone.View-specific state is now stored in a heap-allocated
ViewSpecificStatestruct that is reference counted viastd::shared_ptr. A static map holds astd::weak_ptrto each view-specific state, keyed by the session's file id.SharedCacheretrieves or creates its view-specific state during its constructor.Since
ViewSpecificStateis reference counted it will naturally be deallocated when the lastSharedCacheinstance that references it goes away. Its corresponding entry will remain in the static map, though since it only holds astd::weak_ptrrather than any state it will not use much memory. The next time view-specific state is retrieved any expired entries will be removed from the map.The caching
The second commit moves lookup of type libraries into a function and has it first consult a cache on the view-specific state. The cache stores both found type libraries and the absence of a type library (
nullptr). The type library is only looked up on the view if it's not present in the cache.This was inspired by investigation done by @WeiN76LQh and solves the same problem as their #6195.