Skip to content

Conversation

@mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Jan 17, 2024

I noticed that this project uses the @inline annotation which actually doesn't do anything unless you have the scala-2 inliner enable, quoting from https://docs.scala-lang.org/overviews/compiler-options/optimizer.html

The @inline annotation only has an effect if the inliner is enabled. It tells the inliner to always try to inline the annotated method or callsite.

Ontop of this the PR splits out the implementation between Scala 2 and Scala 3. This is because Scala 3 hasn't ported the Scala 2 optimizer yet. Instead we use the inline keyword which for our intents and purposes achieves the same effect.

MiMa filters were added but this is for code that was marked private/internal use so it shouldn't be an issue.

@mdedetrich mdedetrich marked this pull request as draft January 17, 2024 13:30
@mdedetrich mdedetrich force-pushed the enable-scala2-inliner branch 5 times, most recently from 7eb7019 to e1f3684 Compare January 17, 2024 13:56
@mdedetrich mdedetrich force-pushed the enable-scala2-inliner branch from e1f3684 to bbea2b4 Compare January 21, 2024 23:07
@mdedetrich mdedetrich marked this pull request as ready for review January 21, 2024 23:13
rewrite.rules = [AvoidInfix, SortImports, RedundantBraces, RedundantParens, SortModifiers]
docstrings.style = Asterisk
align.preset = none
project.layout = StandardConvention
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setting instructs scalafmt to apply the Scala 3 dialect to Scala 3 sources since this is the first time that this project has explicit Scala 3 sources.

@rossabaker
Copy link
Member

Is there any empirical evidence that the inliner helps here? Sometimes it even makes things worse.

@mdedetrich
Copy link
Contributor Author

I did some testing but because I am overseas I only had my M1 pro which is notoriously difficult to benchmark especially when detecting minor performance improvements.

Ill be back at home in a few weeks and can re-run benchmarks on an actual desktop with locked clocks, will ping you then

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.

2 participants