Re: RFR (XS) 8224849 : Flag (?U:...) is allowed for non-capturing groups

2019-06-25 Thread Ivan Gerasimov

Thank you Brent for review!


On 6/25/19 10:40 AM, Brent Christian wrote:

That looks fine, Ivan.

-Brent

On 6/24/19 10:46 PM, Ivan Gerasimov wrote:

Hello!

Would someone volunteer to review this extra-small doc-only fix?

Thanks in advance!

With kind regards,

Ivan


On 5/28/19 9:33 AM, Ivan Gerasimov wrote:

Hello!

When the fix for JDK-7039066 (which added support for the flag 
UNICODE_CHARACTER_CLASS and corresponding embedded expression (?U)) 
was integrated back in JDK 7, the documentation for the 
non-capturing groups was not updated accordingly.


Would you please help review the fix, which only updates the javadoc?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8224849
WEBREV: http://cr.openjdk.java.net/~igerasim/8224849/00/webrev/

Thanks in advance!







--
With kind regards,
Ivan Gerasimov



Re: RFR (XS) 8224716 : Javadoc of Int/Long/DoubleSummaryStatistics should mention possible overflow of count

2019-06-25 Thread Brian Burkhalter
Hi Ivan,

> On Jun 25, 2019, at 6:14 PM, Ivan Gerasimov  wrote:
> 
> On 6/25/19 3:33 PM, Joseph D. Darcy wrote:
>> Hello,
>> 
>> Please file a CSR for this change; thanks,
>> 
> 
> Filed: https://bugs.openjdk.java.net/browse/JDK-8226784 
> 
> 
> Please take a look at your convenience.

Reviewed.

Thanks,

Brian

Re: RFR (XS) 8224716 : Javadoc of Int/Long/DoubleSummaryStatistics should mention possible overflow of count

2019-06-25 Thread Ivan Gerasimov

Hello!

On 6/25/19 3:33 PM, Joseph D. Darcy wrote:

Hello,

Please file a CSR for this change; thanks,



Filed: https://bugs.openjdk.java.net/browse/JDK-8226784

Please take a look at your convenience.

With kind regards,
Ivan


-Joe

On 6/25/2019 11:30 AM, Brent Christian wrote:
I was musing about this myself.  If the changes are within the 
@implNote(s), then perhaps not?


-Brent

On 6/25/19 11:15 AM, Brian Burkhalter wrote:
>

Is a CSR in order?

Thanks,

Brian

On Jun 25, 2019, at 5:49 AM, Ivan Gerasimov 
 wrote:


Hello!

Would someone volunteer to review this extra-small doc-only fix?

Thanks in advance!

With kind regards,
Ivan


On 5/29/19 2:39 PM, Ivan Gerasimov wrote:

Hello!

In the implNote section of the javadoc for xxxSummaryStatistics 
classes it is mentioned that the sum is not checked for overflow.
It would be more accurate to add that neither does the 
implementation check the count for overflow.


Would you please help review this doc-only change?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8224716
WEBREV: http://cr.openjdk.java.net/~igerasim/8224716/00/webrev/



--
With kind regards,
Ivan Gerasimov







--
With kind regards,
Ivan Gerasimov



Re: RFR (XS) 8224716 : Javadoc of Int/Long/DoubleSummaryStatistics should mention possible overflow of count

2019-06-25 Thread Joseph D. Darcy

Hello,

Please file a CSR for this change; thanks,

-Joe

On 6/25/2019 11:30 AM, Brent Christian wrote:
I was musing about this myself.  If the changes are within the 
@implNote(s), then perhaps not?


-Brent

On 6/25/19 11:15 AM, Brian Burkhalter wrote:
>

Is a CSR in order?

Thanks,

Brian

On Jun 25, 2019, at 5:49 AM, Ivan Gerasimov 
 wrote:


Hello!

Would someone volunteer to review this extra-small doc-only fix?

Thanks in advance!

With kind regards,
Ivan


On 5/29/19 2:39 PM, Ivan Gerasimov wrote:

Hello!

In the implNote section of the javadoc for xxxSummaryStatistics 
classes it is mentioned that the sum is not checked for overflow.
It would be more accurate to add that neither does the 
implementation check the count for overflow.


Would you please help review this doc-only change?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8224716
WEBREV: http://cr.openjdk.java.net/~igerasim/8224716/00/webrev/



--
With kind regards,
Ivan Gerasimov






Re: RFR: JDK-8224486: Arguments from jpackager cfg file not processed correctly

2019-06-25 Thread Alexander Matveev

Hi Alexey,

I cannot use index to get items from map. At least I did not figure it 
out how to do this. We need to use iterator when getting values from map.


Thanks,
Alexander

On 6/25/2019 4:52 AM, Alexey Semenyuk wrote:
In 
http://cr.openjdk.java.net/~almatvee/8224486/webrev.00/src/jdk.jpackage/share/native/libapplauncher/JavaVirtualMachine.cpp.sdiff.html, 
JavaOptions::AppendValues(), why do we need branching based on the 
result of Values.GetAllowDuplicates()? Why can't we use the same logic 
that was introduced by your patch to read values from the map in which 
duplicate keys are not allowed?


- Alexey

On 6/24/2019 9:18 PM, Alexander Matveev wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- Duplicated Java options were not read correctly from OrderedMap, 
instead we read only unique from map. Fixed by reading duplicated 
Java options.


[1] https://bugs.openjdk.java.net/browse/JDK-8224486
[2] http://cr.openjdk.java.net/~almatvee/8224486/webrev.00/

Thanks,
Alexander






Re: RFR: JDK-8226709: MethodTypeDesc::resolveConstantDesc needs access check per the specification

2019-06-25 Thread Vicente Romero

Thanks Mandy, I will fix this in my local copy,

Vicente

On 6/25/19 10:37 AM, Mandy Chung wrote:

Hi Vicente,

This looks fine.

Nit: the new test line 71 and 80 have an extra space "Error (" that 
can be taken out.


Mandy

On 6/24/19 8:33 PM, Vicente Romero wrote:
Please review fix for [1], at [2]. The implementation of method 
MethodTypeDesc.resolveConstantDes is not in sync with its 
specification. In particular where it says [3]:


Resolves this descriptor reflectively, emulating the resolution 
behavior of JVMS 5.4.3 and the access control behavior of JVMS 5.4.4.
The resolution and access control context is provided by the 
MethodHandles.Lookup parameter.


Thanks,
Vicente

[1] https://bugs.openjdk.java.net/browse/JDK-8226709
[2] http://cr.openjdk.java.net/~vromero/8226709/webrev.00/
[3] 
http://hg.openjdk.java.net/jdk/jdk/file/ae2e53e379cb/src/java.base/share/classes/java/lang/constant/ConstantDesc.java#l89 







Re: RFR (XS) 8224716 : Javadoc of Int/Long/DoubleSummaryStatistics should mention possible overflow of count

2019-06-25 Thread Brent Christian
I was musing about this myself.  If the changes are within the 
@implNote(s), then perhaps not?


-Brent

On 6/25/19 11:15 AM, Brian Burkhalter wrote:
>

Is a CSR in order?

Thanks,

Brian


On Jun 25, 2019, at 5:49 AM, Ivan Gerasimov  wrote:

Hello!

Would someone volunteer to review this extra-small doc-only fix?

Thanks in advance!

With kind regards,
Ivan


On 5/29/19 2:39 PM, Ivan Gerasimov wrote:

Hello!

In the implNote section of the javadoc for xxxSummaryStatistics classes it is 
mentioned that the sum is not checked for overflow.
It would be more accurate to add that neither does the implementation check the 
count for overflow.

Would you please help review this doc-only change?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8224716
WEBREV: http://cr.openjdk.java.net/~igerasim/8224716/00/webrev/



--
With kind regards,
Ivan Gerasimov




Re: RFR (XS) 8224716 : Javadoc of Int/Long/DoubleSummaryStatistics should mention possible overflow of count

2019-06-25 Thread Brian Burkhalter
Hi Ivan,

Looks OK to me. Is a CSR in order?

Thanks,

Brian

> On Jun 25, 2019, at 5:49 AM, Ivan Gerasimov  wrote:
> 
> Hello!
> 
> Would someone volunteer to review this extra-small doc-only fix?
> 
> Thanks in advance!
> 
> With kind regards,
> Ivan
> 
> 
> On 5/29/19 2:39 PM, Ivan Gerasimov wrote:
>> Hello!
>> 
>> In the implNote section of the javadoc for xxxSummaryStatistics classes it 
>> is mentioned that the sum is not checked for overflow.
>> It would be more accurate to add that neither does the implementation check 
>> the count for overflow.
>> 
>> Would you please help review this doc-only change?
>> 
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8224716
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8224716/00/webrev/
>> 
> 
> -- 
> With kind regards,
> Ivan Gerasimov



Re: JPackage EA build 8 ( jdk-14-jpackage+1-8 )

2019-06-25 Thread Scott Palmer
I just tried this out. One thing that has changed since my last testing was the 
“create-installer” vs “--package-type” parameter.

“--package-type all” isn’t allowed.   If I want to create .rpm and .deb, or 
.exe. and .msi, do I need to run twice now?

Maybe this could be a comma separated list or the --package-type parameter 
could be specified multiple times?

This also doesn’t address the issue on macOS where the the typical distribution 
of a .pkg file is in a .dmg image.  On macOS you typically have either an 
Application Bundle (the raw macOS app image) in a .dmg, or an installer package 
(.pkg) in a .dmg.I’ve never seen a .pkg distributed on it’s own, though I 
guess there is no reason it couldn’t be.  Unlike the Application Bundle, an 
installer package really is a single file.

The --help output on macOS ends with the line “Platform dependent options for 
creating the application package:” with no options listed after that.  Are 
there options for setting the background graphic image of the installer package 
and different images for each flavour of .dmg?  The .dmg with the Application 
Bundle should be able to have a different background image than the .dmg with a 
.pkg installer.  The most common contents of the Application Bundle .dmg are a 
alias to the Applications folder and an arrow graphic on the background to 
suggest dragging the app bundle to the Applications folder.  For a installer 
package no such arrow image would be needed, though you may have a graphic with 
some instructions, or just branding imagery.

Regards,

Scott


> On Jun 24, 2019, at 2:31 PM, Andy Herrick  wrote:
> 
> The next EA build of JPackage is available at https://jdk.java.net/jpackage/
> 
> This build ( jdk-14-jpackage+1-8 ) is the first early access release based on 
> JDK-14
> 
> This release contains fixes to the following issues:
> 
> JDK-8225428: CLI change to remove "mode", rename to "package", and build only 
> one target
> JDK-8226191: jpackager --license-file option broken on windows for jdk 
> installers.
> JDK-8225569: jpackage app-image layout
> JDK-8224132: Investigate feasibility of doing SQE code coverage runs for 
> jpackage
> JDK-8225092: Several jpackage tests failes when run with jcov enabled
> JDK-8221333: Replace Inno Setup with custom MSI wrapper for .exe bundler
> JDK-8226193: BundleNameTest and BundleIdentifierTest fails if run without 
> network connection
> JDK-8223643: Provide better defined context for custom installer steps on 
> Windows
> JDK-8223402: Create tests for some Mac installer specific options
> JDK-8225023: JPackageCreateAppImageBundleNameTest fails
> JDK-8223038: JPackage code signing fails on Mac.
> JDK-8223318: jpackage --mac-bundle-name option doesn't work
> JDK-8223080: Build team code review requests.
> JDK-8223212: Code cleanup found during jpackage review
> JDK-8223586: remove jpackage dead code and other cleanup
> JDK-8223953: Fix CLASSPATH parsing for sub-directorys containing spaces
> JDK-8224748: --add-launcher option --add-modules
> JDK-8222901: different behavior when --name option not used
> JDK-8223241: jpackage cleanup from code review
> JDK-822: Use try-with-resources where feasible
> JDK-8224130: create additional automated tests for create-app-image
> JDK-8223334: Additional cleanup in jpackage tool
> JDK-8224116: populate jpackage manual tests for Linux
> JDK-8223700: Create markdown file for jpackage man pages
> JDK-8223189: Fix trailing whitespace and whitespace only file modification.
> JDK-8223321: jpackage ToolProvider is not thread-safe
> JDK-8226532: cleanup is not called when jpackage command fails.
> JDK-8224597: create automated tests for platform create-app-image options
> 
> please send feedback to core-libs-dev@openjdk.java.net
> 
> /Andy Herrick
> 



Re: RFR (XS) 8224849 : Flag (?U:...) is allowed for non-capturing groups

2019-06-25 Thread Brent Christian

That looks fine, Ivan.

-Brent

On 6/24/19 10:46 PM, Ivan Gerasimov wrote:

Hello!

Would someone volunteer to review this extra-small doc-only fix?

Thanks in advance!

With kind regards,

Ivan


On 5/28/19 9:33 AM, Ivan Gerasimov wrote:

Hello!

When the fix for JDK-7039066 (which added support for the flag 
UNICODE_CHARACTER_CLASS and corresponding embedded expression (?U)) 
was integrated back in JDK 7, the documentation for the non-capturing 
groups was not updated accordingly.


Would you please help review the fix, which only updates the javadoc?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8224849
WEBREV: http://cr.openjdk.java.net/~igerasim/8224849/00/webrev/

Thanks in advance!





Re: RFR: JDK-8224486: Arguments from jpackager cfg file not processed correctly

2019-06-25 Thread Andy Herrick

I'm fine with this either way.

The ini file will have GetAllowDuplicates() true, but any OrderedMap 
that doesn't will have count of 1.


/Andy


On 6/25/2019 7:52 AM, Alexey Semenyuk wrote:
In 
http://cr.openjdk.java.net/~almatvee/8224486/webrev.00/src/jdk.jpackage/share/native/libapplauncher/JavaVirtualMachine.cpp.sdiff.html, 
JavaOptions::AppendValues(), why do we need branching based on the 
result of Values.GetAllowDuplicates()? Why can't we use the same logic 
that was introduced by your patch to read values from the map in which 
duplicate keys are not allowed?


- Alexey

On 6/24/2019 9:18 PM, Alexander Matveev wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- Duplicated Java options were not read correctly from OrderedMap, 
instead we read only unique from map. Fixed by reading duplicated 
Java options.


[1] https://bugs.openjdk.java.net/browse/JDK-8224486
[2] http://cr.openjdk.java.net/~almatvee/8224486/webrev.00/

Thanks,
Alexander






Re: RFR: JDK-8226709: MethodTypeDesc::resolveConstantDesc needs access check per the specification

2019-06-25 Thread Mandy Chung

Hi Vicente,

This looks fine.

Nit: the new test line 71 and 80 have an extra space "Error (" that can 
be taken out.


Mandy

On 6/24/19 8:33 PM, Vicente Romero wrote:
Please review fix for [1], at [2]. The implementation of method 
MethodTypeDesc.resolveConstantDes is not in sync with its 
specification. In particular where it says [3]:


Resolves this descriptor reflectively, emulating the resolution 
behavior of JVMS 5.4.3 and the access control behavior of JVMS 5.4.4.
The resolution and access control context is provided by the 
MethodHandles.Lookup parameter.


Thanks,
Vicente

[1] https://bugs.openjdk.java.net/browse/JDK-8226709
[2] http://cr.openjdk.java.net/~vromero/8226709/webrev.00/
[3] 
http://hg.openjdk.java.net/jdk/jdk/file/ae2e53e379cb/src/java.base/share/classes/java/lang/constant/ConstantDesc.java#l89 





JPackage EA build 8 ( jdk-14-jpackage+1-8 )

2019-06-25 Thread Rachel Greenham

core-libs-dev-requ...@openjdk.java.net wrote on 24/06/2019 20:12:

I've been getting the new jpackage working with our project build. 
Generally everything's fine. One probable minor bug relating to 
JDK-8221333:  Replace Inno Setup with custom MSI wrapper for .exe bundler:


The generated EXE installer, when run from eg: file explorer or browser 
downloads dialog, also opens a blank command window behind the 
installer's own window, which I'm pretty sure Inno Setup generated 
installers didn't do. Can this be avoided? (Is there a 
possibly-insufficiently-documented option involved, or is it just a 
bug?) Otherwise it's basically working so it's just a cosmetic 
untidyness. No, the --win-console option is not being used, besides 
which this is the *installer* that's affected, not the actual 
application once installed. This also (probably unsurprisingly) doesn't 
affect the MSI installer.


--
Rachel


Re: RFR (XS) 8224716 : Javadoc of Int/Long/DoubleSummaryStatistics should mention possible overflow of count

2019-06-25 Thread Ivan Gerasimov

Hello!

Would someone volunteer to review this extra-small doc-only fix?

Thanks in advance!

With kind regards,
Ivan


On 5/29/19 2:39 PM, Ivan Gerasimov wrote:

Hello!

In the implNote section of the javadoc for xxxSummaryStatistics 
classes it is mentioned that the sum is not checked for overflow.
It would be more accurate to add that neither does the implementation 
check the count for overflow.


Would you please help review this doc-only change?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8224716
WEBREV: http://cr.openjdk.java.net/~igerasim/8224716/00/webrev/



--
With kind regards,
Ivan Gerasimov



Re: RFR: JDK-8224486: Arguments from jpackager cfg file not processed correctly

2019-06-25 Thread Alexey Semenyuk
In 
http://cr.openjdk.java.net/~almatvee/8224486/webrev.00/src/jdk.jpackage/share/native/libapplauncher/JavaVirtualMachine.cpp.sdiff.html, 
JavaOptions::AppendValues(), why do we need branching based on the 
result of Values.GetAllowDuplicates()? Why can't we use the same logic 
that was introduced by your patch to read values from the map in which 
duplicate keys are not allowed?


- Alexey

On 6/24/2019 9:18 PM, Alexander Matveev wrote:

Please review the jpackage fix for bug [1] at [2].

This is a fix for the JDK-8200758-branch branch of the open sandbox 
repository (jpackage).


- Duplicated Java options were not read correctly from OrderedMap, 
instead we read only unique from map. Fixed by reading duplicated Java 
options.


[1] https://bugs.openjdk.java.net/browse/JDK-8224486
[2] http://cr.openjdk.java.net/~almatvee/8224486/webrev.00/

Thanks,
Alexander




RE: [11u]: Backport of RFR 8211122: Reduce the number of internal classes made accessible to jdk.unsupported (and JDK-8205537 and JDK-8211121)

2019-06-25 Thread Langer, Christoph
Hi,

after Alan's input and contemplating a bit more about this I think we really 
should not pull the removals to 11.

I explored amending the backport of JDK-8211122 to keep the applet classes and 
the ReflectionFactory method working and found out it's quite trivial. So I'll 
go that route. Will post a new webrev after all tests pass.

Thanks,
Christoph

> -Original Message-
> From: Andrew Haley 
> Sent: Dienstag, 25. Juni 2019 10:18
> To: Alan Bateman ; Langer, Christoph
> 
> Cc: AWT-DEV Mailing List ; Java Core Libs
> ; jdk-updates-...@openjdk.java.net
> Subject: Re: [11u]: Backport of RFR 8211122: Reduce the number of internal
> classes made accessible to jdk.unsupported (and JDK-8205537 and JDK-
> 8211121)
> 
> On 6/24/19 4:12 PM, Alan Bateman wrote:
> > On 24/06/2019 15:23, Langer, Christoph wrote:
> >> :
> >> The original issues didn't have CSRs attached but it really feels CSRish. 
> >> Let
> me know whether I shall create CSRs as you're still sorting out the process.
> > The sun.applet package was JDK internal so no CSR required to change or
> > remove anything in that package. That said, we've historically been
> > cautious about changing internal classes too much in update releases,
> > mostly because it was never clear what/who might be using them directly.
> 
> Yes, exactly. There are indeed people providing applet support.
> I don't know if any of it works on 11, though.
> 
> > It's a bit easier since we moved the platform to modules but we aren't
> > yet at the point where the standard and JDK modules are fully
> > encapsulated at run-time.
> >
> > As part of JEP 260 we put sun.reflect.ReflectionFactory into the
> > "Critical internal API" bucket (with sun.misc.Unsafe and a few others)
> > as it provides functionality that custom serialization libraries have
> > been using. I think the CORBA bridge was the main user of
> > newConstructorForSerialization. We neglected to remove the method in
> JDK
> > 11 when removing java.corba module. It was removed in JDK 12 and that
> > change should probably should have had a CSR (we wouldn't remove
> > anything from sun.misc.Signal or sun.misc.Unsafe without a CSR). I'm not
> > involved in JDK updates but I don't think it's a good idea to attempt to
> > remove this in an update release.
> 
> I agree. It'd need a big upside to justify the risk.
> 
> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. 
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671


Re: [11u]: Backport of RFR 8211122: Reduce the number of internal classes made accessible to jdk.unsupported (and JDK-8205537 and JDK-8211121)

2019-06-25 Thread Andrew Haley
On 6/24/19 4:12 PM, Alan Bateman wrote:
> On 24/06/2019 15:23, Langer, Christoph wrote:
>> :
>> The original issues didn't have CSRs attached but it really feels CSRish. 
>> Let me know whether I shall create CSRs as you're still sorting out the 
>> process.
> The sun.applet package was JDK internal so no CSR required to change or 
> remove anything in that package. That said, we've historically been 
> cautious about changing internal classes too much in update releases, 
> mostly because it was never clear what/who might be using them directly. 

Yes, exactly. There are indeed people providing applet support.
I don't know if any of it works on 11, though.

> It's a bit easier since we moved the platform to modules but we aren't 
> yet at the point where the standard and JDK modules are fully 
> encapsulated at run-time.
> 
> As part of JEP 260 we put sun.reflect.ReflectionFactory into the 
> "Critical internal API" bucket (with sun.misc.Unsafe and a few others) 
> as it provides functionality that custom serialization libraries have 
> been using. I think the CORBA bridge was the main user of 
> newConstructorForSerialization. We neglected to remove the method in JDK 
> 11 when removing java.corba module. It was removed in JDK 12 and that 
> change should probably should have had a CSR (we wouldn't remove 
> anything from sun.misc.Signal or sun.misc.Unsafe without a CSR). I'm not 
> involved in JDK updates but I don't think it's a good idea to attempt to 
> remove this in an update release.

I agree. It'd need a big upside to justify the risk.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671


RE: [BUG] Inet6Address.isIPv4CompatibleAddress uses wrong prefix

2019-06-25 Thread Langer, Christoph
Hi Rob,

sending this over to net-dev, where it should be discussed...

/Christoph

> -Original Message-
> From: core-libs-dev  On Behalf
> Of Rob Spoor
> Sent: Montag, 24. Juni 2019 22:58
> To: core-libs-dev@openjdk.java.net
> Subject: [BUG] Inet6Address.isIPv4CompatibleAddress uses wrong prefix
> 
> I found a bug in Inet6Adress.isIPv4CompatibleAddress(). While parsing
> correctly uses the ::: format, isIPv4CompatibleAddress()
> checks for :: instead. An example:
> 
>  Inet6Address address = (Inet6Address)
> InetAddress.getByName("::192.168.1.13");
>  System.out.printf("%s: %b%n", address,
> address.isIPv4CompatibleAddress());
> 
> This should print false, but instead it prints true.
> 
> The error is in the Inet6Address.Inet6AddressHolder class:
> 
>  boolean isIPv4CompatibleAddress() {
>  if ((ipaddress[0] == 0x00) && (ipaddress[1] == 0x00) &&
>  (ipaddress[2] == 0x00) && (ipaddress[3] == 0x00) &&
>  (ipaddress[4] == 0x00) && (ipaddress[5] == 0x00) &&
>  (ipaddress[6] == 0x00) && (ipaddress[7] == 0x00) &&
>  (ipaddress[8] == 0x00) && (ipaddress[9] == 0x00) &&
>  (ipaddress[10] == 0x00) && (ipaddress[11] == 0x00))  {
>  return true;
>  }
>  return false;
>  }
> 
> I think that bytes 10 and 11 should both be (byte) 0xFF instead of 0x00.
> This is what's being used in IPAddressUtil, which is used for parsing:
> 
>  private static boolean isIPv4MappedAddress(byte[] addr) {
>  if (addr.length < INADDR16SZ) {
>  return false;
>  }
>  if ((addr[0] == 0x00) && (addr[1] == 0x00) &&
>  (addr[2] == 0x00) && (addr[3] == 0x00) &&
>  (addr[4] == 0x00) && (addr[5] == 0x00) &&
>  (addr[6] == 0x00) && (addr[7] == 0x00) &&
>  (addr[8] == 0x00) && (addr[9] == 0x00) &&
>  (addr[10] == (byte)0xff) &&
>  (addr[11] == (byte)0xff))  {
>  return true;
>  }
>  return false;
>  }
> 
> Maybe it's an idea to let Inet6Address.Inet6AddressHolder delegate to
> this latter method?
> 
> 
> Rob


Re: RFR: JDK-8222373 Improve CDS performance for custom class loaders

2019-06-25 Thread Remi Forax



- Mail original -
> De: "John Rose" 
> À: "Ioi Lam" 
> Cc: "core-libs-dev" , "hotspot-runtime-dev" 
> 
> Envoyé: Lundi 24 Juin 2019 19:13:04
> Objet: Re: RFR: JDK-8222373 Improve CDS performance for custom class loaders

> On Jun 20, 2019, at 12:12 AM, Ioi Lam  wrote:
>> ...
>> I have a rough idea -- let's have a higher-level representation of the 
>> bytecode
>> stream than byte[] or ByteBuffer, to make optimization possible.
> 
> Template classes will need something similar, so maybe there is a common 
> design
> point in here.
> 
> The key feature of template classes is that their loading sequence is split 
> into
> parts:
> 
> 1. Load the template, defining an abstract API and preprocessing incomplete
> parts.
> 2. Specialize the loaded template, completing all parts, loading a specialized
> class ("species").
> 
> Step 1 happens once per template.  Step 2 can happen any number of types, with
> varying template parameters.

types -> times

> 
> I think that, at least in the internals, there will be a BSM-like (now, 
> that's a
> surprise) template specialization function (TSF) declared in step 1 and
> executed in step 2.

It has to be a BSM-like otherwise we will bolt the Java the language generic 
semantics into the template specialization mechanism.
Given that we want to support any languages generics (even the ones that 
doesn't exist yet), we have to ask how the substitution is done.

> 
> The TSF will need to consult the results of step one and request the JVM to
> combine them into the desired species.  Ad hoc logic may be involved, such as
> detecting if T in List is Comparable and mixing in Comparable into the
> resulting List, with a standard comparison method (e.g. lexicographic compare
> lifted from the elemental compares).
> 
> Key questions:
> 
> A. What API reflects the template chunks loaded in step 1?  Probably not just
> byte streams, more like what you are reaching for here, Ioi.
> 
> B. What API loads the customized chunks?  Probably (again) not today's
> ClassLoader which requires byte streams.
> 
> I'm slowly working on an answer to A, in the form of a "class excavator" which
> pulls out structural information from live JVM metadata, without "deadening" 
> it
> into a byte stream.  Not ready for prime time, but the basic idea is to 
> present
> the logical schema of the loaded class file (in terms of the original 
> classfile
> structure) in a style similar to the existing constant pool reflection API
> (which is JDK internal).

yes, once we have live metadata, we can not go back and try to shove them into 
the byte stream, otherwise there will be a kind of "runtime erasure".

> 
> Once we get a fuller answer for A we can try to invert it to get an answer for
> B.  That is, if we know what we'd like to see in the JVM (via the excavator) 
> we
> want to tell the JVM to establish some new species definition, related to the
> excavated data.
> 
> For most JDK work we can wrap low-level excavator/establisher mechanisms 
> (which
> don't need to be very O-O) in a thin layer of value types and interfaces.
> 
> I'm using new terms to help me think of this as a new kind of API, not just a
> small variation on class loading or reflection.  The new feature is that the
> reflective part comes first, and is followed (as a sort of reversed operation)
> by the loading part.  Also new is that we don't want to abstract everything
> through serialized byte streams, since they make it very hard to share
> features—but (I think) species need to share template metadata, not just make
> copies of it.

other open questions:
 - what are the metadata we need to expose, constant pool constants, obviously, 
but also vtable slot and field layout slot ?
 - what are the checks performed by the VM we a live value is stored as 
metadata, a covariant checks seems to be the answer ? 

> 
> — John

Rémi

> 
>> So we could have a new API in ClassLoader
>> 
>> protected final Class defineClass(String name, String location,
>> ProtectionDomain protectionDomain)
>> 
>> and its behavior is equivalent to the following
>> 
>>{
>> byte[] b = read_buffer_from(location);
>> return defineClass(name, b, 0, b.len, protectionDomain);
>>}
>> 
>> 
>> examples would be:
>> 
>>  defineClass("java/lang/Object", 
>> "jrt:/java.base/java/lang/Object.class", NULL);
>>  defineClass("com/foo/Bar", "file://path/com.foo.jar!com/foo/Bar.class",
>>  myProtectionDomain);
>> 
>> Note that the type and value of  is just for illustrative 
>> purposes. We
>> might use a different type (URI??). It's just a way to name a location that 
>> you
>> can read a byte buffer from.
>> 
>> The protectionDomain will need to be created by the caller. The use of the
>> protectionDomain will be no different than the existing defineClass() APIs.
>> Specifically, it will not be used in any way to fetch the buffer.
>> 
>> When CDS is enabled, the VM can check if the name+location matches a class in
>> the CDS archi