search

drym-org / qi

An embeddable flow-oriented language.
59 stars 12 forks source link

Simplify deps #51

Closed benknoble closed 2 years ago

benknoble commented 2 years ago

Summary of Changes

If @countvajhula has time to test that this makes a difference for #50 that would be great.

Public Domain Dedication

(Why: The freely released, copyright-free work in this repository represents an investment in a better way of doing things called attribution-based economics. Attribution-based economics is based on the simple idea that we gain more by giving more, not by holding on to things that, truly, we could only create because we, in our turn, received from others. As it turns out, an economic system based on attribution -- where those who give more are more empowered -- is significantly more efficient than capitalism while also being stable and fair (unlike capitalism, on both counts), giving it transformative power to elevate the human condition and address the problems that face us today along with a host of others that have been intractable since the beginning. You can help make this a reality by releasing your work in the same way -- freely into the public domain in the simple hope of providing value. Learn more about attribution-based economics at drym.org, tell your friends, do your part.)

countvajhula commented 2 years ago

Lightspeed, man. I will test this soon 🙂

countvajhula commented 2 years ago

This is what I get:

BEFORE:
real    0m0.850s
real    0m0.807s
real    0m0.894s
real    0m0.830s
real    0m0.812s

AFTER:
real    0m0.804s
real    0m0.787s
real    0m0.795s
real    0m0.798s
real    0m0.802s
> (~> (0.850 0.807 0.894 0.830 0.812) (-< + count) /)

0.8386000000000001

> (~> (0.804 0.787 0.795 0.798 0.802) (-< + count) /)

0.7972

Looks like about a 40ms or about 5% improvement - not a bad start 😄

benknoble commented 2 years ago

Wow, I was expecting a bigger difference. I'm assuming you ran raco setup?

countvajhula commented 2 years ago

Yeah me too! But yes, I did run raco setup. I wonder what is causing the majority of the latency then, if not mischief. I’ll try running this test for some other libraries and see what times look like.

Sent from my iPhone

On Jul 10, 2022, at 4:33 PM, D. Ben Knoble @.***> wrote:

Wow, I was expecting a bigger difference. I'm assuming you ran raco setup?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

countvajhula commented 2 years ago

I wrote a small CLI script that accepts a module name at the command line and then:

Run it as e.g.:

$ ./loadlib.rkt rebellion

The times on packages I tested ranged 0-4 s, with almost all of them in the range 0-500 ms, and Qi being on the higher end at ~600ms, as reported.

mischief came in heavy at 570 ms, while mischief/shorthand by itself was 150 ms, explaining why it wasn't contributing as much as we thought. Adjutor is about 200 ms, so you'd think it would trim that much off the (require qi) loading time, so, not sure why it only trimmed < 40 ms. I think we should expect 150 + 200 = 350 ms savings from leaving these out, no?

Interestingly enough, rebellion appears to have no overhead over racket/base at all, not sure how @notjack managed this magic!

benknoble commented 2 years ago

@countvajhula after the tests pass (🤞) could you try profiling this again? I expect this to make a bigger difference now that the deps for qi-lib are slimmed as much as possible.

benknoble commented 2 years ago

Heh, when installing the package takes 6+ minutes but running the tests takes 14s… (the CS current build for the "remove typed-stack dependency" commit https://github.com/countvajhula/qi/pull/51/commits/11d5ec8562a28ec1d18258aadf004e12db4b8ada)

countvajhula commented 2 years ago

So, yes, basically, a factor of 3 improvement. No big deal at all 😆

Here's the full data from running the scripts.

  1. First, the dependency breakdown using loadprof from @Bogdanp :

Before:

$ ./loadprof.rkt qi/flow
...
Timings:
* /Users/siddhartha/work/lisp/racket/qi/qi-lib/flow.rkt (274.30ms)
  * ...Users/siddhartha/work/lisp/racket/qi/qi-lib/private/util.rkt (246.39ms)
    * (submod ...ddhartha/Library/Racket/8.3/pkgs/typed-stack/typed-stack.rkt #%contract-defs) (237.96ms)
    * ...sers/siddhartha/Library/Racket/8.3/pkgs/typed-stack/main.rkt (208.26ms)
    * /Users/siddhartha/Library/Racket/8.3/pkgs/adjutor/main.rkt (19.47ms)
      * ...a/Library/Racket/8.3/pkgs/adjutor/stable/require-provide.rkt (11.39ms)
      * ...dhartha/Library/Racket/8.3/pkgs/adjutor/stable/serialize.rkt (10.34ms)
      * ...ary/Racket/8.3/pkgs/adjutor/stable/environment-variables.rkt (7.02ms)
      * ...artha/Library/Racket/8.3/pkgs/adjutor/stable/define-star.rkt (6.15ms)
  * /Users/siddhartha/Library/Racket/8.3/pkgs/adjutor/main.rkt (19.47ms)
    * ...a/Library/Racket/8.3/pkgs/adjutor/stable/require-provide.rkt (11.39ms)
    * ...dhartha/Library/Racket/8.3/pkgs/adjutor/stable/serialize.rkt (10.34ms)
    * ...ary/Racket/8.3/pkgs/adjutor/stable/environment-variables.rkt (7.02ms)
    * ...artha/Library/Racket/8.3/pkgs/adjutor/stable/define-star.rkt (6.15ms)
  * /Users/siddhartha/work/lisp/racket/qi/qi-lib/macro.rkt (6.39ms)

After:

$ ./loadprof.rkt qi/flow
...
Timings:
* /Users/siddhartha/work/lisp/racket/qi/qi-lib/flow.rkt (11.31ms)
  * /Users/siddhartha/work/lisp/racket/qi/qi-lib/macro.rkt (5.83ms)
  1. The dependency breakdown using the -l option, which shows the proportion of total time contributed by the leaves in the dependency tree, sorted by the ones taking up the most time:

Before:

Timings:
* (submod ...ddhartha/Library/Racket/8.3/pkgs/typed-stack/typed-stack.rkt #%contract-defs) (248.77ms, weight 0.341886)
* ...sers/siddhartha/Library/Racket/8.3/pkgs/typed-stack/main.rkt (221.54ms, weight 0.304462)
* ...Users/siddhartha/work/lisp/racket/qi/qi-lib/private/util.rkt (183.36ms, weight 0.251991)
* /Users/siddhartha/Library/Racket/8.3/pkgs/adjutor/main.rkt (19.92ms, weight 0.027375)
* ...dhartha/Library/Racket/8.3/pkgs/adjutor/stable/serialize.rkt (10.32ms, weight 0.014179)
* ...a/Library/Racket/8.3/pkgs/adjutor/stable/require-provide.rkt (8.13ms, weight 0.011178)
* /Users/siddhartha/work/lisp/racket/qi/qi-lib/macro.rkt (6.30ms, weight 0.008651)
* ...ary/Racket/8.3/pkgs/adjutor/stable/environment-variables.rkt (5.35ms, weight 0.007348)

After:

Timings:
* /Users/siddhartha/work/lisp/racket/qi/qi-lib/macro.rkt (6.46ms, weight 0.257713)
  1. Using the hyperfine utility recommended by @samth , the naive cost of racket -l qi is:

Before:

$ ./hyperfine "racket -l qi"
Benchmark 1: racket -l qi
  Time (mean ± σ):     672.5 ms ±   5.9 ms    [User: 544.3 ms, System: 122.0 ms]
  Range (min … max):   664.5 ms … 682.3 ms    10 runs

After:

$ ./hyperfine "racket -l qi"
Benchmark 1: racket -l qi
  Time (mean ± σ):     347.1 ms ±   3.6 ms    [User: 261.7 ms, System: 80.0 ms]
  Range (min … max):   343.5 ms … 354.0 ms    10 runs

While this is statistically rigorous, it does not factor out the cost of racket/base, so it doesn't tell us the true extent of improvement from these changes. So:

  1. Finally, the resulting output from loadlib which is basically a descendent of @sarna 's method for measuring the cost of (require qi):

Before: $ ./loadlib.rkt qi (run five times):

500.0 ms
500.0 ms
500.0 ms
510.0 ms
520.0 ms

After: $ ./loadlib.rkt qi (run five times):

170.0 ms
170.0 ms
160.0 ms
170.0 ms
180.00000000000003 ms

I'd say that's pretty darn convincing.

benknoble commented 2 years ago

Love it! Thanks for doing the analysis to confirm my suspicions :)

These are valuable tools to have in the toolbox for my own work

countvajhula commented 2 years ago

Yeah definitely. I'm wondering what the best way to collect these kinds of tools might be so that they're discoverable. Either a repo containing them all, or an "awesome X" style repo containing curated links, or ...?

In any case, this LGTM. Feel free to merge it if you think it's good to go :)

benknoble commented 2 years ago

I'm not planning on doing any more (e.g., installing the repo takes a while because of the deps for docs and profiling and such, but I'm not going to tackle that today and it may not be worth tackling), so since you gave the green-light I will indeed merge this.

Bogdanp commented 2 years ago

Just a heads-up that I've turned the gist into a proper package:

countvajhula commented 2 years ago

@sarna Any interest in maintaining something resembling loadlib.rkt as a package similar to Bogdan's racket-import-profiler above? I think it would be very similar in structure so a lot of the config could probably mirror racket-import-profiler almost verbatim. If not, I'm happy to maintain it as well, lmk.

sarna commented 2 years ago

@countvajhula Nope, sorry! I don't have the time :(

countvajhula commented 2 years ago

I've put up the loadlib script as a package as well:

countvajhula commented 2 years ago

@benknoble re: build times on CI, I've migrated the upstream relation library to lib/test/doc and trimmed its dependencies. I think this should reduce the times since I suspect a good chunk of it is from depending on relation (used in Qi's tests and docs), where we can now use relation-lib instead. I'll push the change on the main branch soon and see what we end up with 🤞

countvajhula commented 2 years ago

Da-ang, look at those build times 🐎 The total time is down from about 14 mins to about 9 mins, with individual builds going from about 6 mins down to about 3 mins. [edit: for the complete picture, the time had already reduced from 17+ total mins a little while back when we transitioned to using #:indirect links in the docs]

There were many changes going on concurrently so I'm not completely sure what this is attributable to, but here are some of the things I can think of:

benknoble commented 2 years ago

If I may just say, using "sdk" in the name of this extra package was genius.