RFR: 8213481: [REDO] Fix incorrect copy constructors in hotspot
Kim Barrett
kim.barrett at oracle.com
Tue Nov 27 01:36:59 UTC 2018
> On Nov 26, 2018, at 7:24 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
Thanks for looking at this.
> I am fine with changes for copy constructor.
>
> Do we have other cases where it is used? I think the original use in [1] could be removed because I doubt the hack will have performance benefits for such big methods as stated in comment. May be we would not need it copy constructor then.
There are other uses. For example, StackMapFrame gets copied (see
StackMapFrame::copy). That's why I deleted the "Used in" comment.
I don't have an informed opinion about the benefit (or lack thereof)
of the copy in parse_constant_pool_entries. This copy is using the
implicit copier for the stream class, but is being allocated on the
stack so the old copier worked.
> For assignment operator I have reservation. You can't use original allocation type if you can't guaranty that destination is the same type.
I'm not sure what you mean here. The assignment destination was
allocated somewhere, and the assignment doesn't change where it was
allocated. And that location has nothing to do with where the
assignment source was allocated. So copying the allocation information
would be wrong, and was not being done by the old code; I didn't
change that.
What I changed was removing the assertion that the assignment
destination was stack/member allocated, since I don't think that's a
correct or reasonable assumption (though apparently there aren't any
counter examples in current usage).
> Thanks,
> Vladimir
>
> http://hg.openjdk.java.net/jdk/jdk/file/3db8758f0f79/src/hotspot/share/classfile/classFileParser.cpp#l135
>
> On 11/18/18 10:14 PM, Kim Barrett wrote:
>> […] CR:
>> https://bugs.openjdk.java.net/browse/JDK-8213481
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8213481/open.00/
>> Testing:
>> mach5 tier1-3. There were many failures in these tiers with just the
>> addition of the missing copy constructor calls (JDK-8213414).
More information about the hotspot-dev
mailing list