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
