Add a task to show statisticts about serialized size/source size for the top 100 gems #1532
Conversation
|
It's a lot better with #1450: With that serialized size is ~77% of source size on average. |
eregon
force-pushed
the
serialized-size-stats
branch
2 times, most recently
from
9abd366
to
2b02b4e
Compare
eregon
force-pushed
the
serialized-size-stats
branch
from
2b02b4e
to
0ec7688
Compare
| @@ -170,6 +170,12 @@ jobs: | |||
| run: bundle exec rake lex:topgems | |||
| - name: Parse Top 100 Gems | |||
| run: bundle exec rake parse:topgems | |||
| - name: Serialized size stats with all fields | |||
There was a problem hiding this comment.
This is an interesting metric, but I don't think we want it to run on every CI run. The topgems check is already one of the longest-running. I'm fine keeping the task around though.
There was a problem hiding this comment.
This only takes like 2 seconds to execute, so I think it has no observable impact on CI time.
There was a problem hiding this comment.
To be precise, the 3 commands below respectively take, locally on my machine: 2s, 6s, 2s.
I'd like to keep this check in CI because it's a very convenient way to check if serialized size got better or worse or same for a PR.
There was a problem hiding this comment.
On this CI run https://github.com/ruby/yarp/actions/runs/6207594674/job/16852941189 it's 3+19+3 seconds.
There was a problem hiding this comment.
I wonder if rake lex:topgems could be optimized further, that takes 4m22s in that run.
There was a problem hiding this comment.
A quick profile with stackprof/autorun gives:
stackprof /tmp/stackprof20230919-104519-jjlzxp.dump
==================================
Mode: wall(1000)
Samples: 21520 (90.63% miss rate)
GC: 13603 (63.21%)
==================================
TOTAL (pct) SAMPLES (pct) FRAME
8441 (39.2%) 8441 (39.2%) (marking)
7520 (34.9%) 7520 (34.9%) (sweeping)
5530 (25.7%) 5530 (25.7%) Thread#join
3 (0.0%) 3 (0.0%) Kernel#require
15963 (74.2%) 2 (0.0%) (garbage collection)
5530 (25.7%) 0 (0.0%) Array#map
7890 (36.7%) 0 (0.0%) YARP.parallelize
...
Which looks strange, like non-main threads are not included or so.
That does seem like a lot of GC though.
Also ENV.fetch("WORKERS") { 16 } is probably way too high for GitHub Actions, maybe it could use Etc.nprocessors or so, or it might be more efficient to not parallelize that at all on CRuby (might just increase GVL contention).
There was a problem hiding this comment.
@kddnewton I can move this to a separate CI job, then it won't make this one any longer. Would that address your concern?
eregon
force-pushed
the
serialized-size-stats
branch
from
0ec7688
to
291177e
Compare
|
I've rebased on main so we can see the actual values with |
cc @enebo
Currently it's:
total serialized/total source is slightly better than in #741 (reply in thread) which was 0.91 (for
protobuf)However the new stats reveal than on average and median, serialized size > source size (by ~8%), which is kind of unexpected.
So likely some files are big and have many comments or so and then for those serialized size is much smaller than source size and that could explain the total sizes being better for serialized.
Note this is without #1450 (so with all location fields), I'll post the stats with that here later.