Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]
On Wed, 2 Mar 2022 00:32:46 GMT, Quan Anh Mai wrote: >> Quan Anh Mai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> the other > > Thanks a lot for your reviews, do I need a second review for this? > @ExE-Boss IMO generally we only decorate `ForceInline` on functions of which > inlining is crucial since inlining decision is a hard problem and often not > solvable from the local point of view, especially from the callee's one. @merykitty it's good. - PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]
On Tue, 1 Mar 2022 02:22:49 GMT, Quan Anh Mai wrote: >> Hi, >> >> `Objects.requireNonNull` may fail to be inlined. The call is expensive and >> may lead to objects escaping to the heap while the null check is cheap and >> is often elided. I have observed this when using the vector API when a call >> to `Objects.requireNonNull` leads to vectors being materialised in a hot >> loop. >> >> Should the other `requireNonNull` be `ForceInline` as well? >> >> Thank you very much. > > Quan Anh Mai has updated the pull request incrementally with one additional > commit since the last revision: > > the other Thanks a lot for your reviews, do I need a second review for this? @ExE-Boss IMO generally we only decorate `ForceInline` on functions of which inlining is crucial since inlining decision is a hard problem and often not solvable from the local point of view, especially from the callee's one. - PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]
On Tue, 1 Mar 2022 02:22:49 GMT, Quan Anh Mai wrote: >> Hi, >> >> `Objects.requireNonNull` may fail to be inlined. The call is expensive and >> may lead to objects escaping to the heap while the null check is cheap and >> is often elided. I have observed this when using the vector API when a call >> to `Objects.requireNonNull` leads to vectors being materialised in a hot >> loop. >> >> Should the other `requireNonNull` be `ForceInline` as well? >> >> Thank you very much. > > Quan Anh Mai has updated the pull request incrementally with one additional > commit since the last revision: > > the other Marked as reviewed by psandoz (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]
On Tue, 1 Mar 2022 02:22:49 GMT, Quan Anh Mai wrote: >> Hi, >> >> `Objects.requireNonNull` may fail to be inlined. The call is expensive and >> may lead to objects escaping to the heap while the null check is cheap and >> is often elided. I have observed this when using the vector API when a call >> to `Objects.requireNonNull` leads to vectors being materialised in a hot >> loop. >> >> Should the other `requireNonNull` be `ForceInline` as well? >> >> Thank you very much. > > Quan Anh Mai has updated the pull request incrementally with one additional > commit since the last revision: > > the other I think `@ForceInline` should also be applied to the overload that takes the message supplier, and probably also `requireNonNullElse` and `requireNonNullElseGet`. - PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]
On Tue, 1 Mar 2022 02:22:49 GMT, Quan Anh Mai wrote: >> Hi, >> >> `Objects.requireNonNull` may fail to be inlined. The call is expensive and >> may lead to objects escaping to the heap while the null check is cheap and >> is often elided. I have observed this when using the vector API when a call >> to `Objects.requireNonNull` leads to vectors being materialised in a hot >> loop. >> >> Should the other `requireNonNull` be `ForceInline` as well? >> >> Thank you very much. > > Quan Anh Mai has updated the pull request incrementally with one additional > commit since the last revision: > > the other I have applied the change for the other one as well. Thanks a lot. - PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]
> Hi, > > `Objects.requireNonNull` may fail to be inlined. The call is expensive and > may lead to objects escaping to the heap while the null check is cheap and is > often elided. I have observed this when using the vector API when a call to > `Objects.requireNonNull` leads to vectors being materialised in a hot loop. > > Should the other `requireNonNull` be `ForceInline` as well? > > Thank you very much. Quan Anh Mai has updated the pull request incrementally with one additional commit since the last revision: the other - Changes: - all: https://git.openjdk.java.net/jdk/pull/7543/files - new: https://git.openjdk.java.net/jdk/pull/7543/files/bcd22b9d..b362bdca Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7543=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7543=00-01 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7543.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7543/head:pull/7543 PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline
- Original Message - > From: "Paul Sandoz" > To: "core-libs-dev" > Sent: Tuesday, March 1, 2022 1:48:02 AM > Subject: Re: RFR: 8282143: Objects.requireNonNull should be ForceInline > On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai wrote: > >> Hi, >> >> `Objects.requireNonNull` may fail to be inlined. The call is expensive and >> may >> lead to objects escaping to the heap while the null check is cheap and is >> often >> elided. I have observed this when using the vector API when a call to >> `Objects.requireNonNull` leads to vectors being materialised in a hot loop. >> >> Should the other `requireNonNull` be `ForceInline` as well? >> >> Thank you very much. > > `Objects.requireNonNull` is also used on the critical path of VarHandles, and > other places in `j.l.invoke` so I think this is generally beneficial beyond > use > by the Vector API. It is also use by javac when the JLS requires to emit a nullcheck, like for example when creating a method reference. > > - > > PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline
On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai wrote: > Hi, > > `Objects.requireNonNull` may fail to be inlined. The call is expensive and > may lead to objects escaping to the heap while the null check is cheap and is > often elided. I have observed this when using the vector API when a call to > `Objects.requireNonNull` leads to vectors being materialised in a hot loop. > > Should the other `requireNonNull` be `ForceInline` as well? > > Thank you very much. `Objects.requireNonNull` is also used on the critical path of VarHandles, and other places in `j.l.invoke` so I think this is generally beneficial beyond use by the Vector API. - PR: https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline
On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai wrote: > Hi, > > `Objects.requireNonNull` may fail to be inlined. The call is expensive and > may lead to objects escaping to the heap while the null check is cheap and is > often elided. I have observed this when using the vector API when a call to > `Objects.requireNonNull` leads to vectors being materialised in a hot loop. > > Should the other `requireNonNull` be `ForceInline` as well? > > Thank you very much. Very simple methods like this are almost always inlined, but I suppose there are depth cutoffs that may cause inlining to fail. Yes, it should always be inlined. The optimizer works hard to track null-ness conditions, and making this call go out of line "breaks the thread of thought" in the optimizer's reasoning about null. Yes, both versions of `O::cNN` should be marked this way; the only difference is in how the exception is constructed. (A similar point goes for index-checking functions, which are also force-inlined so that simple index checks of the form `i*K+L https://git.openjdk.java.net/jdk/pull/7543
Re: RFR: 8282143: Objects.requireNonNull should be ForceInline
On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai wrote: > Should the other `requireNonNull` be `ForceInline` as well? Probably yes. - PR: https://git.openjdk.java.net/jdk/pull/7543
RFR: 8282143: Objects.requireNonNull should be ForceInline
Hi, `Objects.requireNonNull` may fail to be inlined. The call is expensive and may lead to objects escaping to the heap while the null check is cheap and is often elided. I have observed this when using the vector API when a call to `Objects.requireNonNull` leads to vectors being materialised in a hot loop. Should the other `requireNonNull` be `ForceInline` as well? Thank you very much. - Commit messages: - requireNonNull force inline Changes: https://git.openjdk.java.net/jdk/pull/7543/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7543=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282143 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7543.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7543/head:pull/7543 PR: https://git.openjdk.java.net/jdk/pull/7543
