> It will only do that in some select few cases. Agreed, I phrased the original statement incorrectly. I was meant to say that it's not going to pointlessly inline code, i.e. it will only inline code that is known to be not inlinable by earlier JDK's. Its of course impossible to inline any/all hypothetical code
> See https://github.com/apache/incubator-pekko-http/pull/456 for an example of the correctness issue. Afaik none of the many projects enable the inliner for development. By correctness I was referring to the inliner producing incorrect code on a clean compile. Yes it's known that it has issues when done on local development due to incremental compilation which is why there is a knob to disable it for local dev. There is already an issue[1] about reversing the default setting so that it's disabled by default. It probably wasn't a wise idea to have it enabled by default locally (my bad) but the original motivation for that was because we can't make release artifacts in CI yet, it was to reduce the chance of creating an incorrect release artifact however as stated below this is likely a non-issue. > If we decide to run the inliner on CI and for releases, then this will introduce another level of complexity to important steps of the release pipeline building differently in different situations. Our release process regardless of inliner is already complex enough[2] and hence we are already having to pass custom flags[3] when creating release artifacts anyways so I don't see how adding an extra single flag i.e. pekko.inline=true to sbt is going to materially affect the complexity either way. It's also already document to +clean the project for reasons aside from the inliner, i.e. the incremental compiler even with inliner disabled doesn't produce deterministic bytecode which effect's reproducible builds which is something we are trying to move to. [1]: https://github.com/pjfanning/sbt-pekko-build/issues/9#issuecomment-1914496929 [2]: https://github.com/apache/incubator-pekko-site/wiki/Pekko-Release-Process [3]: https://github.com/apache/incubator-pekko-site/wiki/Pekko-Release-Process#deploy-the-jars-to-apache-maven-repository-staging On Mon, Jan 29, 2024 at 9:31 PM Johannes Rudolph <johannes.rudo...@gmail.com> wrote: > On Thu, Jan 18, 2024 at 12:48 AM Matthew de Detrich > <matthew.dedetr...@aiven.io.invalid> wrote: > > it will automatically inline any code that it knows won't get inlined by > the JDK > > It will only do that in some select few cases. > > > In any case, I would still opt for leaving the inliner in there. I would > be > > extremely > > shocked if there are any correctness issues given that its being used for > > at least > > half a decade in many projects > > See https://github.com/apache/incubator-pekko-http/pull/456 for an > example of the correctness issue. > Afaik none of the many projects enable the inliner for development. > > > hence the "free lunch" argument and the > > complexity is not that high (we are dealing with some extra scalac flags) > > I would not call it free lunch, especially since the development > efficiency is impacted by far longer compile times and potential > correctness issues. > If we decide to run the inliner on CI and for releases, then this will > introduce another level of complexity to important steps of the > release pipeline building differently in different situations. > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@pekko.apache.org > For additional commands, e-mail: dev-h...@pekko.apache.org > > -- Matthew de Detrich *Aiven Deutschland GmbH* Immanuelkirchstraße 26, 10405 Berlin Alexanderufer 3-7, 10117 Berlin Amtsgericht Charlottenburg, HRB 209739 B Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen *m:* +491603708037 *w:* aiven.io *e:* matthew.dedetr...@aiven.io
