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

[WIP] binary bloat analysis #1223

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pauldreik
Copy link
Member

@pauldreik pauldreik commented Oct 10, 2020

This is for measuring binary size. Not that anyone asked about it that I know of, but it may be interesting to

  • see which functions take up space
  • notice if the binary size suddenly increases

It is really quick to run, less than a minute.

However, I do not really know how to present the result and/or possibly act on it, see djarek/bloaty-analyze#1

@jkeiser
Copy link
Member

jkeiser commented Oct 12, 2020

It's definitely a good idea to keep track of size over time ... that affects the ability to distribute.

Not part of this at ALL, but someday it'll be nice to do a size comparison--instead of just comparing simdjson against other libraries for speed on certain tasks, make a separate executable for each library+task combo and compare size as well.

@pauldreik
Copy link
Member Author

Would a reasonable small example be one that takes the twitter json and outputs the first tweet found that contains "cats" ?

I think there are several metrics interesting to keep track of over time

  • lines of code in total
  • fuzz coverage (I have a manually updated table here)
  • unit test coverage
  • performance (N platforms x M benchmarks)
  • binary size of the library
  • binary size of "hello world" similar to what @jkeiser proposes above

There was discussion of tracking performance earlier, perhaps it would be possible to store all these metrics somewhere?

The curl project tracks all sorts of things over time, we might get inspiration there.

@jkeiser
Copy link
Member

jkeiser commented Dec 22, 2020

For some reason I didn't see your response: I think it's reasonable to run this on simdjson.so at a minimum, and perhaps the parse executable.

For ondemand, perhaps the partial_tweets benchmarks? Perhaps benchmark_ondemand as a whole, which would potentially give us interesting comparisons.

@jkeiser
Copy link
Member

jkeiser commented Mar 20, 2021

@pauldreik I'm fine leaving this pr around if you plan to get back to it; otherwise let's file an issue and get back to it when we have time :)

@pauldreik
Copy link
Member Author

@pauldreik I'm fine leaving this pr around if you plan to get back to it; otherwise let's file an issue and get back to it when we have time :)

I pinged the author of the bloaty action job, let's see if I get a response and if not, let's do as you suggest!

@pauldreik
Copy link
Member Author

@jkeiser I fixed the bloaty job and it seems to work, but what should we do with the results? should we reject the pull request if the binary size increases more than X%?

@jkeiser
Copy link
Member

jkeiser commented Jun 24, 2021

@pauldreik do you have any idea whether the results of this are generally stable? If so, I think it'd be reasonable to reject changes with a 20% size change (or at least flag the crap out of them). @lemire thoughts?

At the very, very least, we should run this in CI so we can go look at the results when we're worried. If it's expensive we can restrict to just master pushes.

@lemire
Copy link
Member

lemire commented Jun 24, 2021

@jkeiser Sure, sure.

@pauldreik
Copy link
Member Author

pauldreik commented Jun 27, 2021 via email

@jkeiser jkeiser changed the title DON'T MERGE binary bloat analysis [WIP] binary bloat analysis Jun 28, 2021
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.

None yet

3 participants