Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad wrote: > As an alternative to #7667 I took a look at injecting an empty class array > from the VM. Turns out we already do this for exception types - see > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 > - and we can do similarly for the parameter types array. We still need to > parse the signature for the return type, though. > > I've verified by dumping and inspecting heaps that this means we are not > allocating extra `Class[]` on `Method` reflection. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad wrote: > As an alternative to #7667 I took a look at injecting an empty class array > from the VM. Turns out we already do this for exception types - see > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 > - and we can do similarly for the parameter types array. We still need to > parse the signature for the return type, though. > > I've verified by dumping and inspecting heaps that this means we are not > allocating extra `Class[]` on `Method` reflection. Looks good. - Marked as reviewed by coleenp (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Wed, 6 Apr 2022 09:42:21 GMT, Claes Redestad wrote: > Do I need a second reviewer for the hotspot changes? FWIW, I think hotspot changes are quite simple, so maybe no need for second reviewer? - PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Wed, 6 Apr 2022 07:58:33 GMT, Aleksey Shipilev wrote: >> As an alternative to #7667 I took a look at injecting an empty class array >> from the VM. Turns out we already do this for exception types - see >> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 >> - and we can do similarly for the parameter types array. We still need to >> parse the signature for the return type, though. >> >> I've verified by dumping and inspecting heaps that this means we are not >> allocating extra `Class[]` on `Method` reflection. > > Looks fine. I would have thought to put `assert(parameter_count > 0)` near > `mirrors->obj_at_put(index++, mirror);`, but I think it is fine as it is, > since `obj_at_put` does its own bounds checking. Thanks for reviewing! As @shipilev point out `obj_at_put` would assert if we tried to put anything into an empty array, so it seems redundant to add more checks here. Perhaps a comment to point that out since it was brought up here. Do I need a second reviewer for the hotspot changes? - PR: https://git.openjdk.java.net/jdk/pull/8089
Re: RFR: 8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method
On Mon, 4 Apr 2022 09:58:35 GMT, Claes Redestad wrote: > As an alternative to #7667 I took a look at injecting an empty class array > from the VM. Turns out we already do this for exception types - see > https://github.com/openjdk/jdk/blob/master/src/hotspot/share/oops/method.cpp#L918 > - and we can do similarly for the parameter types array. We still need to > parse the signature for the return type, though. > > I've verified by dumping and inspecting heaps that this means we are not > allocating extra `Class[]` on `Method` reflection. Looks fine. I would have thought to put `assert(parameter_count > 0)` near `mirrors->obj_at_put(index++, mirror);`, but I think it is fine as it is, since `obj_at_put` does its own bounds checking. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8089
