-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix benchmarks and add multiple mini benchmark tests #1690
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
Conversation
Review @felixmulder and @DarkDimius ? |
@@ -74,11 +76,24 @@ object BenchTests extends OnlineRegressionReport { | |||
def setup = | |||
performance of "dotty" in { | |||
measure.method("stdlib") in { | |||
using(Gen.unit("test")) curve "stdlib" in { r => stdLib } | |||
// maybe scalac curve later | |||
using(Gen.unit("test")) curve "dotty" in { r => stdLib } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this curve called dotty
now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DarkDimius The curve
is to support multiple curves in a chart. Yes, I should keep it the same for stdlib
and dotty-src
for backward data compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DarkDimius the name for stdlib
and dotty-src
are now restored.
Status? |
@DarkDimius This is one is up to you to review, thanks :) |
LGTM |
The benchmarks has been down due to some weird error. A single line change fix it: link
And I've added several mini tests which covers implicit search, exhaustivity check, tailrec, etc. We can just put any mini test to the folder
bench/tests
, it will be automatically tracked in the benchmarks.