RFR: 8237878: Improve ModuleLoaderMap datastructures
forax at univ-mlv.fr
forax at univ-mlv.fr
Thu Feb 6 17:49:42 UTC 2020
----- Mail original -----
> De: "Claes Redestad" <claes.redestad at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Jeudi 6 Février 2020 14:48:38
> Objet: Re: RFR: 8237878: Improve ModuleLoaderMap datastructures
> 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.
>> 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
>> 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.
If your variable have a name like resolvedModule then the type ResolvedModule doesn't give you more info.
Obviously, it doesn't work if the variable name is an abbreviation, but instead of not using var, i think it's better to change the variable name.
> New webrev:
> Re-running some tests..
>> 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.
>> ----- 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
>>> 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
More information about the core-libs-dev