brew icon indicating copy to clipboard operation
brew copied to clipboard

[WIP] Load internal json v3

Open apainintheneck opened this issue 1 year ago • 3 comments

  • [x] Have you followed the guidelines in our Contributing document?
  • [x] Have you checked to ensure there aren't other open Pull Requests for the same change?
  • [x] 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.
  • [x] Have you successfully run brew style with your changes locally?
  • [x] Have you successfully run brew typecheck with your changes locally?
  • [ ] Have you successfully run brew tests with your changes locally?

This is an early PR to start loading the internal JSON v3 representation added in https://github.com/Homebrew/brew/pull/16541. The hope is that this will not only load faster than the previous one but also will reduce the amount of data that has to flow across the network and reduce the amount of code needed to turn the current JSON representation into the expected shape internally.

My first thought is that the amount of code needed to get this to work is way less than I originally thought. Of course, no tests so far so this will grow a bit.

  • https://github.com/Homebrew/brew/issues/16410

apainintheneck avatar Feb 11 '24 07:02 apainintheneck

Early benchmarks on my super old iMac show that this is noticeably faster. I'll try it on a newer machine at some point to get more relevant information. Note that HOMEBREW_INTERNAL_JSON_V3 just gets ignored on the master branch.

$ hyperfine --parameter-list branch master,load-internal-json-v3 --warmup 3 --setup 'git switch {branch}' 'HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl'
Benchmark 1: HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = master)
  Time (mean ± σ):      3.164 s ±  0.020 s    [User: 2.086 s, System: 0.915 s]
  Range (min … max):    3.133 s …  3.196 s    10 runs

Benchmark 2: HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = load-internal-json-v3)
  Time (mean ± σ):      2.731 s ±  0.020 s    [User: 1.694 s, System: 0.877 s]
  Range (min … max):    2.710 s …  2.783 s    10 runs

Summary
  HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = load-internal-json-v3) ran
    1.16 ± 0.01 times faster than HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = master)

apainintheneck avatar Feb 12 '24 05:02 apainintheneck

Early benchmarks on my super old iMac show that this is noticeably faster. I'll try it on a newer machine at some point to get more relevant information. Note that HOMEBREW_INTERNAL_JSON_V3 just gets ignored on the master branch.

Wow, that's huge! Great work @apainintheneck!

MikeMcQuaid avatar Feb 12 '24 15:02 MikeMcQuaid

Okay, I finally got around to adding some tests. There are tests for both generating the JSON format and loading formulae from the JSON file. I ended up creating a mini-tap fixture to facilitate testing as well. I assume grabbing a few random formulae for that is not a big deal. Let me know if there's anything else that would be good to test for.

Edit: I guess I'll be debugging the cross-platform test failures for a bit...

apainintheneck avatar Feb 19 '24 04:02 apainintheneck

This should be ready for review again. The test failures were caused by caching problems between test runs. I just needed to clear the caches a bit more between them and add one more variable to the Tap.clear_cache method. https://github.com/Homebrew/brew/pull/16710 solved some of those problems though which was helpful. https://github.com/Homebrew/brew/pull/16671 could help solve these sorts of problems more in the future.

I'm leaving this as draft until after the Monday release since I'd like to be cautious and a have a few days before it's available to everyone to avoid any unexpected bugs.

apainintheneck avatar Feb 25 '24 18:02 apainintheneck

Here are the same benchmarks on my M1 MacBook that I use at work. Still significant performance boost though not as big as the one on my older computer. Percentage-wise it's more significant though.

$ hyperfine --parameter-list branch master,load-internal-json-v3 --warmup 3 --setup 'git switch {branch}' 'HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl'
Benchmark 1: HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = master)
  Time (mean ± σ):      1.079 s ±  0.010 s    [User: 0.738 s, System: 0.164 s]
  Range (min … max):    1.070 s …  1.096 s    10 runs

Benchmark 2: HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = load-internal-json-v3)
  Time (mean ± σ):     876.8 ms ±  10.4 ms    [User: 554.8 ms, System: 146.4 ms]
  Range (min … max):   867.7 ms … 895.6 ms    10 runs

Summary
  HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = load-internal-json-v3) ran
    1.23 ± 0.02 times faster than HOMEBREW_INTERNAL_JSON_V3=1 brew info jrnl (branch = master)

apainintheneck avatar Feb 27 '24 06:02 apainintheneck

I'll merge this in tomorrow.

apainintheneck avatar Feb 28 '24 05:02 apainintheneck