scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

build should error when jar size for library/reflect/compiler exceeds certain threshold #11578

Closed adriaanm closed 5 years ago

adriaanm commented 5 years ago

Backstop to prevent future instances of unexpected jar growth, as reported in #11577

ashawley commented 5 years ago

To inform what the threshold check could be, here's the historical sizes using unix tools on my Ivy cache for scala-library. The byte difference for each release is with the the major version (e.g. 2.11.0, 2.12.0):

Scala library Size in megs Size Δ in kb
2.11.0 5.3
2.11.1 5.3 -36
2.11.2 5.3 -32
2.11.4 5.3 -40
2.11.5 5.3 0
2.11.6 5.3 -12
2.11.7 5.5 164
2.11.8 5.5 164
2.11.9 5.5 184
2.11.11 5.5 168
2.11.12 5.5 168
2.12.0 5.2
2.12.1 5.0 -172
2.12.2 5.0 -168
2.12.3 5.0 -200
2.12.4 5.0 -196
2.12.5 5.0 -180
2.12.6 5.0 -180
2.12.7 5.0 -168
2.12.8 5.0 -172

Here's what happened with milestones and release candidates during 2.12. The difference is between each release.

Scala library Size in megs Size Δ in kb
2.12.0-M1 5.5
2.12.0-M2 5.0 -564
2.12.0-M3 5.4 388
2.12.0-M4 5.0 -408
2.12.0-M5 4.4 -620
2.12.0-RC1 4.3 -32
2.12.0-RC2 5.2 892
2.12.0 5.2 0

Here's what happened during 2.13 development:

Scala library Size in megs Size Δ in kb
2.13.0-M1 4.1
2.13.0-M2 4.1 -12
2.13.0-M3 4.1 -12
2.13.0-M4 3.8 -292
2.13.0-M5 3.9 96
2.13.0-RC1 5.3 1480
2.13.0-RC2 5.4 112
2.13.0-RC3 5.4 -4
2.13.0 5.4 0
ashawley commented 5 years ago

It would seem you'd want to trigger an error if the change is greater than 100 kb or something. Although, it is possible to imagine that the artifact size could slowly grow to 100 kb after a few dozen merged patches, and then the threshold would be reached suddenly but it would arbitrarily punish that contributor. If the threshold is made too small, then it constantly needs to be changed which would probably cause conflicts in git.

Should it emit a warning if the size was different or beyond some smaller threshold and an error if it was above some greater threshold?

dwijnand commented 5 years ago

Yeah, that's why I was considering a comparison against the latest "mergly" publishes. I'm not sure if adding an early warning would help as, in my experience, nothing gets done until it's a hard failure... But maybe we should try anyway.

adriaanm commented 5 years ago

Thanks! I think it should be an error in the same way mima reports are. We can always change the setting for an expected one, just like for expected mima breakage.

The main goal for me is to catch unexpected sudden changes. Slow creep is also undesirable, but not the priority. For that, we should chart.

I propose “currentArtifactSizeKb” and “maxRelativeDifferencePercent” (and set it to 3?)

dwijnand commented 5 years ago

Updated https://github.com/scala/scala/pull/8171