RFR: JEP 359-Records: reflection code

[Maurizio Cimadamore]

Ok, so the goal is share of CP entries (should be documented somewhere).

I think in that case I would at least advocate for replacing the 
compiler generated string of names with some runtime concatenation using 
getter names; the concatenation is done at runtime, after all.

Currently the code uses 'String::split' in the bootstrap which is not 
great, e.g. sometimes it inserts additional empty strings in the middle 
( e.g. "foo;;bar"), and it's probably as expensive (if not more) as 
constructing the string from scratch (since it involves regex matching). 
I note that the bootstrap code protects against lengths mismatches with 
an 'assert', but the assert will not be run in production code, so there 
could be some surprising crashes there.

If the code is kept this way I suggest at least replacing assert with a 
runtime exception (throw new AssertionError). E.g. I'm fine with assert 
keywords when it comes to checking invariants that are under control of 
the implementation (in which case, you run tests with assertion enabled 
to shake possible issues); but in this case the invariant depends on 
what names string the client has passed to the BSM, so I think some real 
validation is in order here.

Maurizio


On 22/10/2019 03:40, Vicente Romero wrote:
>
>
> On 10/21/19 6:32 PM, Maurizio Cimadamore wrote:
>>
>> On 21/10/2019 23:02, Remi Forax wrote:
>>> - the same call works for equals/hashCode and toString, so depending 
>>> on the methods some arguments are not used or not
>>
>> I too found this a bit odd when looking at the javac code - it took 
>> me sometime to figure out what was going on.
>>
>> The question I have here is - is the decision of having a single 
>> point a deliberate decision that we think it's going to pay off in 
>> the long run? 
>
> it was a deliberate decision, the first implementation actually had 
> different bootstrap methods but then we got a table with three 
> bootstrap methods per every humble record, so having only one 
> bootstrap method is a space gain.
>> Or was it more a way to quickly bootstrap the javac implementation? 
>> If the latter, we should probably revert to one bootstrap per Object 
>> method.
>>
>> Maurizio
>>
> Vicente