Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SharedCache] Cache type libraries #6196

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

bdash
Copy link
Contributor

@bdash bdash commented Nov 25, 2024

Type libraries are surprisingly expensive to look up via BinaryView. Caching them can speed up FindSymbolAtAddrAndApplyToAddr significantly.

  1. 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 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 the session's file id. SharedCache retrieves or creates 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.

  2. 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.

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.
@0cyn
Copy link
Member

0cyn commented Dec 10, 2024

Thanks for the PR! I'll look into getting this merged sometime over the next couple of weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
File Format: SharedCache Issue with the dyld_shared_cache plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants