RFR: 8241371: Refactor and consolidate package_from_name

[Claes Redestad]

Hi Ioi,

On 2020-03-22 23:45, Ioi Lam wrote:
> Hi Claes,
> 
> This is a good optimization. Looks good to me. Here are some minor nits:
> 
> [1] ClassLoader::search_module_entries may be called twice by the same 
> caller,
>      I think it's better to compute the mod_entry in the caller, so you 
> can avoid
>      the pkg_entry lookup twice.

ClassLoader::search_module_entries would only be called twice if
running with --patch-module on an exploded build, which doesn't seem
like a case we should bother to optimize for.


> 
> [2] For tracking the code changes, I think it's better to move your new 
> version
>      of InstanceKlass::package_from_name() to 
> ClassLoader::package_from_name().
> 
>      I did a diff of the new version of 
> InstanceKlass::package_from_name() and
>      the old version of ClassLoader::package_from_name(), and I can see 
> that the
>      logic has not changed (the only changes are how we allocate the 
> buffers).
> 
>      With the current patch, it's hard to see what has changed and 
> difficult to
>      validate its correctness.

Ok, I just felt that as a static utility it's more coupled with a 
class/InstanceKlass than its class loader. If you insist I'll move it
to ClassLoader instead.

> 
>      Also, it's better to rename the function to
>      ClassLoader::package_from_class_name()

Sure, that sounds better!

/Claes

> 
> Thanks
> - Ioi
> 
> 
> 
> On 3/22/20 2:22 PM, Claes Redestad wrote:
>> Correct webrev: http://cr.openjdk.java.net/~redestad/8241371/open.00/
>> Bug:            https://bugs.openjdk.java.net/browse/JDK-8241371
>>
>> On 2020-03-22 17:15, Claes Redestad wrote:
>>> Hi,
>>>
>>> JDK-8153858 partially consolidated the logic shared between
>>> InstanceKlass::package_from_name and ClassLoader::package_from_name.
>>> Unfortunately it also caused an undetected regression in some paths due
>>> need to copy from Symbol -> resource arrays and back again.
>>>
>>> By refactoring package_from_name into a single implementation which
>>> operates on a Symbol* rather than a char* we can both simplify a bit
>>> and regain some lost performance:
>>>
>>> Testing: tier1-5, verified a ~0.5% improvement on startup
>>>
>>> Thanks!
>>>
>>> /Claes
>