On Tue, 30 Jan 2024, 23:14 PJ Fanning, <fannin...@apache.org> wrote: > I would favour disabling the inlining until at least after the 1.1.0-M1 > release. > > Things are moving to the stage where we don't want to enable inlining > except in CI builds. Our releases are not done using CI. The releases > are not going to be automated for a while due the need for us to use > developer keys and the fact that we can't put those in a CI env. If > our releases can't have inlining, I don't see why our CI builds, > including the publishing of our snapshot jars, should have inlining. >
Actually what's been happening is the exact opposite, all of our testing for M1 is being done on the inlined variant and not on the inline one. I am also running both pekko and pekko-http nightly snapshots in some systems and haven't noticed any regression. So suddenly releasing with a non inlined M1 is a drastic change. If you referring to the Scala 3 inline keyword that's an entirely different topic/problem. So changing it now is actually the more drastic option and the main reason for adding the inliner in published snapshots earlier was so extensive testing could be done on it. The last thing I want to do is to just enabled inliner last minute when release is occurring. I don't have an issue in reverting the change, I am saying let's do it when it's clear there is an obvious issue. The issue that Johannes pointed out with local dev is a clear one that I agree with and I have already been doing this change in other projects. > At the end of the day, I think we should wait until we can automate > the releases until we consider adding back inlining. > > On Tue, 30 Jan 2024 at 11:27, Johannes Rudolph > <johannes.rudo...@gmail.com> wrote: > > > > On Tue, Jan 30, 2024 at 8:30 AM Matthew de Detrich > > <matthew.dedetr...@aiven.io.invalid> wrote: > > > > > > > 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. > > > > This is also not how it works. It has some heuristics about when it > > thinks inlining might at least not hurt and is safe to do, but this is > > pretty much not connected theoretically to any existing JDK > > implementation. > > > > You have seen the diff. 90% of all the optimizer improvements is > > trivial pruning of useless basic blocks and trying to remove redundant > > STORE/LOAD pairs. Then there's a tiny bit of extra inlining which only > > works for trivial methods (like the onPull from above). There seem to > > be few places where it works for the intended purpose of optimizing > > HOF for collection usage (sometimes the HOF is inlined but not the > > closure). In other cases where HOF are inlined, these trivial cases > > lead to massive code blow-up which will rather have a negative impact > > on JIT code parsing with the result of the exact same code being > > generated (because these cases are so trivial). In almost every case > > the inliner makes trivial changes that would better be done by the > > JIT. > > > > It's far from clear whether enabling the inliner will create overall > > better code or if it will even put a hit on JIT compilation because of > > the increased code size. It's definitely not a free guaranteed win. > > > > > > 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. > > > > I guess our major disagreement is about priorities. I highly > > prioritize my time as a (for this project mostly unpaid) developer. A > > 2x-3x hit on compilation times is huge in such a big project that > > often accidentally triggers major recompilation. > > > > If you think about it in general, it's far more important for this > > project to remove hurdles for potential contributors than to squeeze > > out the last bits of performance (which is unclear anyway). > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@pekko.apache.org > > For additional commands, e-mail: dev-h...@pekko.apache.org > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@pekko.apache.org > For additional commands, e-mail: dev-h...@pekko.apache.org > >
