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

Implemented different way of persisting data for linkage cache using `pstore`. #1

Closed
wants to merge 2 commits into from

Conversation

@armcburney
Copy link
Owner

commented Jan 24, 2018

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew tests with your changes locally?

This is an experimental branch to look at the performance of using pstore over SQLite3 as a mechanism of caching brew linkage information. It was recommended in Homebrew#3720 I try a pstore implementation to compare the performance of the two caching mechanisms.

Performance Comparison

The pstore implementation is slower than the original brew linkage command without any caching. Storing and fetching data into pstore seems to be an expensive operation.

I've attached a flame graph to showcase the time spent in the LinkageStore::update! and LinkageStore::fetch_path_values!/LinkageStore::fetch_hash_values! methods.

Flame Graph

With the caching the command (instrumented starting from linkage.rb) took 42.2 seconds to complete. One fetch_path_values! method call took 3.51 seconds.

screen shot 2018-01-24 at 12 44 25 pm

Note: I could refactor the LinkageStore class so that the number of database transactions is reduced, improving performance. I wanted to keep the same public interface for LinkageStore when experimenting with the changes. However, since the time it takes to perform one read is greater than the total time for the SQLite3 implementation, I didn't refactor and investigate.

Raw Output

pstore caching solution

time brew linkage docker-machine mysql
==> Checking docker-machine linkage
System libraries:
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /usr/lib/libSystem.B.dylib
==> Checking mysql linkage
System libraries:
  /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
  /usr/lib/libSystem.B.dylib
  /usr/lib/libc++.1.dylib
  /usr/lib/libedit.3.dylib
  /usr/lib/libsasl2.2.dylib
Homebrew libraries:
  /usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib (openssl)
  /usr/local/opt/openssl/lib/libssl.1.0.0.dylib (openssl)
brew linkage docker-machine mysql  39.28s user 3.37s system 98% cpu 43.144 total

SQLite3 caching solution

time brew linkage docker-machine mysql
==> Checking docker-machine linkage
System libraries:
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /usr/lib/libSystem.B.dylib
==> Checking mysql linkage
System libraries:
  /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
  /usr/lib/libSystem.B.dylib
  /usr/lib/libc++.1.dylib
  /usr/lib/libedit.3.dylib
  /usr/lib/libsasl2.2.dylib
Homebrew libraries:
  /usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib (openssl)
  /usr/local/opt/openssl/lib/libssl.1.0.0.dylib (openssl)
brew linkage docker-machine mysql  0.34s user 0.17s system 92% cpu 0.549 total

Recommendation

I would recommend we use SQLite3 for the caching mechanism. I believe the tradeoff of installing another gem in homebrew is worth the performance benefits of using SQLite3 for caching.

If the homebrew team decides to go with the SQLite3 implementation, I'd like to clean up the PR a bit. While writing this experimental branch, I noticed some ways I could refactor my existing code to make it cleaner.

@armcburney armcburney referenced this pull request Jan 24, 2018
6 of 6 tasks complete
@armcburney

This comment has been minimized.

Copy link
Owner Author

commented Jan 24, 2018

The PR has conflicts because I ended up refactoring my SQLite3 implementation branch. I do not plan on merging this PR. The purpose of the PR is to show the differences in the pstore branch, and to contrast the performance differences in the two implementations.

@armcburney armcburney closed this Jan 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.