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] Optimize parsing and applying of slide info #6321

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

bdash
Copy link
Contributor

@bdash bdash commented Jan 15, 2025

Slide info is parsed and applied on the main thread, below the SharedCache constructor, when a shared cache is opened. Time spent applying the slide is time that the main thread is blocked. This collection of optimizations cuts the time spent applying the slide roughly in half (125ms to 65ms on a macOS shared cache).

The individual changes are:

  1. Avoid building the rewrites vector unless SLIDEINFO_DEBUG_TAGS is enabled. Building the vector was the single most expensive operation in the function.
  2. Read slide pointers via ReadULong rather than the variable-length Read method. The compiler wasn't able to eliminate the call to memcpy in the variable-length Read function, and the overhead of calling memcpy is non-trivial given the small size of the read and number of times it is called.
  3. Update ParseAndApplySlideInfoForFile to take a pointer rather than a std::shared_ptr. This method doesn't need to manipulate the ownership of the object so the shared_ptr is unnecessary.

After these changes the profile shows 80% the time spent below ParseAndApplySlideInfoForFile is inside MappedFileAccessor::ReadULong / MappedFileAccessor::WritePointer.

Slide info is parsed and applied on the main thread, below the
`SharedCache` constructor, when a shared cache is opened. Time spent
applying the slide is time that the main thread is blocked. This
collection of optimizations cuts the time spent applying the slide
roughly in half (125ms to 65ms on a macOS shared cache).

The individual changes are:
1. Avoid building the rewrites vector unless `SLIDEINFO_DEBUG_TAGS` is
   enabled. Building the vector was the single most expensive operation
   in the function.
2. Read slide pointers via `ReadULong` rather than the variable-length
   `Read` method. The compiler isn't able to eliminate the call to
   `memcpy` in the variable-length `Read` function, and the function
   call overhead is significant given the small size of the read and
   number of times it is called.
3. Update `ParseAndApplySlideInfoForFile` to take a pointer rather than
   a `std::shared_ptr`. This method doesn't need to manipulate the
   ownership of the object so the `shared_ptr` is unnecessary.
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