RFR: 8237878: Improve ModuleLoaderMap datastructures

[Claes Redestad]

Hi Rémi,


On 2020-02-06 14:08, Remi Forax wrote:
> Hi Claes,
> In ArchivedModuleGraph, there is no point to take the mainModule as parameter given that it should always be null.

right, I've been thinking this should be simplified for now. Once we
implement support for archiving non-default scenarios the internal
datastructure will need a refactoring anyhow, so let's cut things down
to size.

> 
> In ModuleLoaderMap, the constants PLATFORM_LOADER_INDEX and APP_LOADER_INDEX should be moved inside the nested class Mapper given that there are only needed by the Mapper
> and mappingFunction should be moved inside the class Mapper too.

Done.

> 
> in mappingFunction, i think that if you are using 'var', you should using everywhere in the method,
> to avoid to have a weird mix between local variables declared with and without var.
> so either map is not declared with var or resolvedModule and nm are declared with var.

I disagree var usage should be an all or nothing ordeal, rather it
should be used when the type information is readily available, e.g.,
by being stated in detail on the RHS.

The exact type in the other cases would be somewhat muddied: maybe
name() is "obviously" a String, but what type the elements in
cf.modules() are is not at all obvious, and making code less obvious
should be avoided.

New webrev:

http://cr.openjdk.java.net/~redestad/8237878/open.01/

Re-running some tests..

/Claes

> 
> I believe that at some point in the future, let say just before the release of 17, we should do a global pass and rewrite each class to use 'var', the same way we have done with generics.
> It will be nasty for backporting bugs but i think it's better than having a mix between codes that use var and codes that doesn't use it.
> 
> Rémi
> 
> ----- Mail original -----
>> De: "Claes Redestad" <claes.redestad at oracle.com>
>> À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
>> Envoyé: Jeudi 6 Février 2020 13:37:59
>> Objet: RFR: 8237878: Improve ModuleLoaderMap datastructures
> 
>> Hi,
>>
>> refactor ModuleLoaderMap to allow the datastructure to be
>> archived, then archive it.
>>
>> Webrev: http://cr.openjdk.java.net/~redestad/8237878/open.00/
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8237878
>>
>> Testing: tier1-3
>>
>> /Claes