Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

[Paul Sandoz]

Hi Steve,

> On 8 Apr 2016, at 03:57, Steve Drach  wrote:
> 
> I’ve put up a new webrev that addresses the issue of having spaces before 
> (and after) the value “true” in the Multi-Release attribute.
> 

Is some or all of that really necessary? since the we can specify domain of 
values.

For example, the Sealed attribute can take on a value of “true” or “false” 
(case ignored), but there is no white space trimming.

Paul.

>> Hi,
>> Please review this simple fix to require that the jar manifest Multi-Release 
>> attribute has a value of “true" in order to be effective, i.e. to assert the 
>> jar is a multi-release jar.
>> issue: https://bugs.openjdk.java.net/browse/JDK-8153213 
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>> 
>> Thanks
>> Steve

Re: JDK 9 proposal: allocating ByteBuffers on heterogeneous memory

[Paul Sandoz]

Hi Steve,

> On 8 Apr 2016, at 00:03, Dohrmann, Steve  wrote:
> 
> Hi Paul,
> 
> We would like to have an an API for Intel's 3D XPoint memory sooner than the 
> JDK 10 timeframe and proposed this API because it seems simple enough to 
> consider for JDK 9.  As you suggest, we will participate in the Panama 
> discussions  in this area.  Any additional guidance you have would be 
> appreciated.
> 

e.g. clone/build the panama forest, join the pamana-dev email list, and start 
asking questions :-) I can send you links etc. off-line if need be.


> Just to clarify, it is incidental that the proposed Memory interface has only 
> one method.  We see the value of the interface as nominative; a new type that 
> can be passed around to abstract various sources of ByteBuffer memory.
> 

I suspected as much, but would prefer that we gain more experience on what this 
interface should be, and how it intersects with other efforts, rather than 
introducing a skeletal version now.

I suppose it’s possible for such an interface to extend from IntFunction for 
compatibility if existing 8 or 9 dependent libraries use IntFunction for 
abstracting buffer allocation.

FWIW at one point i did postulate and prototyped a MemoryRegion class but after 
some thought and feedback made a hasty retreat :-)


> Regarding construction and allocation, our current Memory implementation 
> allocates ByteBuffers by calling the NewDirectByteBuffer JNI function with a 
> pointer to 3D XPoint memory allocated via a supporting native library.

Ok, that’s what we thought when some of us had an off-line discussion about 
this. It slots in quite nicely, and can be easily abstracted by 
IntFunction i.e. i don’t think there is anything fundamentally 
stopping you providing something that would work on Java 8 or 9.


Is there any documentation on the memory ordering properties of 3D XPoint 
memory? how would it differ from say normal memory? can one access the memory 
using AVX instructions? does it support unaligned loads/stores? (i am guessing 
the latter is yes).

(Separately there is also the question of whether this kind of memory is 
something HotSpot itself could leverage.)


>  The Linux libraries we have worked with are NVML 
> (https://github.com/pmem/nvml/) and memkind 
> (https://github.com/memkind/memkind).  We recently also became aware of the 
> NVM-Direct library (https://github.com/oracle/NVM-Direct).

So do you create JNI bindings to NVML?

Opportunistically perhaps this is also somewhere Panama might be able to help 
with, since it will provide good improvements over the current JNI experience.

Paul.

> We currently don't need our own subclass and return the ByteBuffer returned 
> by the JNI call.
> 
> Thanks,
> Steve
> 

RFR 8151706: Update VarHandle implementation to use @Stable arrays

[Paul Sandoz]

Hi,

Please review:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151706-VH-form-table-stable/webrev/
 


Now that @Stable arrays are supported by C1 (thanks Vladimir!) we can switch 
from the explicit use of MemberName fields in VarForm to a @Stable MemberName[] 
array.

I also took the opportunity to simplify the linking from a VarHandle impl to 
MemberName[] array, now that the implementation has settled down. This will 
reduce initialization costs and memory churn.


I held off making further improvements for now. For example, VarForm can 
probably go away (also removing the dependency on ClassValue). A VarHandle 
instance can directly hold a MemberName[] array whose source reference is 
statically held on the associated VarHandle implementation. It should also be 
possible to lazily create MemberName instances as required, rather than 
aggressively doing so, which may further reduce initialization costs and memory 
usage in common cases.

Paul.

Re: RFR [9] 8153737: Unsupported Module

[Paul Sandoz]

> On 7 Apr 2016, at 19:14, Chris Hegarty  wrote:
> 
> Enough technical debt has been paid down that we can now create the new
> JDK-specific module as proposed by JEP 260 [1], named jdk.unsupported.
> This module will initially contain, and export, the sun.misc package,
> and will eventually export the sun.reflect package too ( once it has
> been sanitized ). The jdk.unsupported module will be present in full JRE
> and JDK images.
> 

And in the modular world it’s necessary to explicitly declare a requirement on 
idk.unsupported.


> http://cr.openjdk.java.net/~chegar/8153737/
> https://bugs.openjdk.java.net/browse/JDK-8153737
> 

+1

An important milestone.

Paul.

JDK 9 RFR of JDK-8153722: Mark java/nio/channels/Selector/SelectAndClose.java as intermittently failing

[Amy Lu]

java/nio/channels/Selector/SelectAndClose.java

This test is known to fail intermittently (JDK-8152814). This patch is 
to mark the test accordingly with keyword 'intermittent'.


bug: https://bugs.openjdk.java.net/browse/JDK-8153722
webrev: http://cr.openjdk.java.net/~amlu/8153722/webrev.00/

Thanks,
Amy

--- old/test/java/nio/channels/Selector/SelectAndClose.java 2016-04-08 
18:36:17.0 +0800
+++ new/test/java/nio/channels/Selector/SelectAndClose.java 2016-04-08 
18:36:17.0 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,6 +23,7 @@
 
 /* @test

  * @bug 5004077
+ * @key intermittent
  * @summary Check blocking of select and close
  */
 

RFR: JDK-8152115 (proxy) Examine performance of dynamic proxy creation

[Peter Levart]

Hi,

With Mandy we have prepared the following patch to restore performance 
of java.lanf.reflect.Proxy class caching that has regressed with 
introduction of additional checks that have to be performed due to modules:


http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.caching/webrev.05/

Jigsaw changed caching of proxy classes  from ClassLoader+interfaces to 
Module+interfaces to accommodate possible future extension of the API 
that might take given Module as a parameter. Additional validation of 
constraints and derivation of dynamic module to host proxy class was 
performed on the fast path, degrading performance. This change restores 
the caching back to ClassLoader+interfaces, but also introduces new 
caching mechanism that allows simple derivation of other coexisting 
caching schemes that can support possible additions to the Proxy API. 
The new mechanism also simplifies caching by avoiding the need to 
reference ClassLoader and interfaces through WeakReference objects. 
Instead it uses a single ConcurrentHashMap per ClassLoader instance 
referenced directly from ClassLoader object. The caching API - 
ClassLoaderValue - is general enough to be used for other purposes where 
caching per ClassLoader is desired. Currently it is encapsulated as a 
java.lang.reflect package-private API, but could simply be promoted to a 
public API in an internal concealed package if desired.


Performance of Proxy class caching is back to pre-jigsaw state and even 
better:


http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.caching/ProxyBench.java


@Mandy

I haven't yet removed the superfluous checking and providing access from 
dynamic module to the modules/packages of transitive closure of 
interfaces implemented by proxy class. I think this should be done in a 
separate issue so that this change doesn't change the semantics of 
implementation.


Regards, Peter

Re: PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries

[KUBOTA Yuji]

Hi Sherman and all,

Thank you for all comments about this proposal!

Thank Tomoyuki and Stephen for sharing your needs.

Thank Bernd for sharing your information. However, I am
afraid that AE1 and AE2 may have a risk of patent issue,
while "Traditional PKWare encryption" is explicitly free
from the risk according to the PKWare documents.
Therefore, I think this proposed implementation to support
legacy zip specification is meaningful as Tomoyuki said.

We got some positive opinions / needs. I am glad if you
consider the proposal acceptable for JDK.
If this proposed implementation is acceptable, I and
Yasumasa will improve it by corresponding to comments
which given by Sherman and Stephen.

Thanks!
Yuji

2016-03-29 6:41 GMT+09:00 Xueming Shen :
> Yasumasa
>
> It's a tricky call. To be honest, as I said at the very beginning, I'm not
> sure whether
> or not it's a good idea and worth the efffort to push this into the j.u.zip
> package to
> support the "traditional PKWare encryption", which is known to be "seriously
> flawed,
> and in particular is vulnerable to known-plaintext attacks" (from wiki),
> while I fully
> understood it is not a concern in your "problem-free" use scenario. Just
> wonder
> if there is anyone else on the list that has/had the need for such
> encryption feature
> in the past. It would be preferred to have more input (agree, disagree) to
> make the
> final decision.
>
> Here are some comments regarding the proposed implementation,
>
> (1) The changes in native code are probably no longer needed. The native
> path is
>  left there for vm directly access.
> (2) I'm concerned about the footprint increase for the ZipEntry class (the
> proposed
>  change is adding 3 more fields into the entry class). Given this is
> something rarely
>  needed/used, and we might have hundreds and thousands of zip/jar
> entries during
>  startup/runtime, it might be desired to avoid adding these fields into
> ZipEntry class.
>  A possible alternative to have a dedicated entry class extended from
> the ZipEntry
>  to hold those extra fields and methods, EncryptionZipEntry for example.
> So the
>  proposed encryption support code will not have any impact to the
> existing use of
>  ZipInput/OutputStream/File.
> (3) I'm also not comfortable to add the "encryption engine" logic into the
> in/deflater
>  stream classes, while it might be convenient to do so from
> implementation point
>  of view. Given the encryption is something between the raw bites in the
> zip file
>  and the in/deflating layer, it might need an extra layer there, though
> I'm not sure
>  if it's easy to do so.
>
> The webrev below is what I think might be better for the ZipFile and
> ZipOutputStream
> to have an extra layer in between to handle the encryption. Not try to test
> if it really
> works, just throw in the idea for discussion.
>
> http://cr.openjdk.java.net/~sherman/zencrypt/webrev/
>
> No, I did not try the ZIS, the "pushback" stream there might be a little
> tricky:-)
>
> -Sherman
>
>
> On 03/28/2016 02:34 AM, Yasumasa Suenaga wrote:
>>
>> PING: Could you review it?
>> We want to move forward this enhancement.
>>
>> Thanks,
>>
>> Yasumasa
>> 2016/03/19 22:01 "Yasumasa Suenaga":
>>
>>> Hi Alan,
>>>
 I think the main issue here is to decide whether the API
 should be extended for encryption or not.
>>>
>>> We've discussed on the premise that we add the API for supporting ZIP
>>> encryption.
>>> In this context, Sherman tried to implement AES encryption extending
>>> current API. [1]
>>>
>>> IMHO, ZIP encryption should be implemented as extention of current ZIP
>>> API
>>> because encryption/decryption will process for each ZIP entries.
>>>
>>> According to Developers' Guide, guideline for adding new API is TBD. [2]
>>> What should I do next?
>>>
>>>
>>> Thanks,
>>>
>>> Yasumsa
>>>
>>>
>>> [1]
>>>
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/037903.html
>>> [2] http://openjdk.java.net/guide/changePlanning.html#api
>>>
>>>
>>> On 2016/03/19 0:25, Alan Bateman wrote:

 On 18/03/2016 15:02, Yasumasa Suenaga wrote:
>
> Hi all,
>
> We (including me and Yuji Kubota (ykubota: OpenJDK jdk9 Author))
>>>
>>> discussed
>
> about this issue from Nov. 2015. [1]
> We heard several comments and we applied them to our patch.
>
> I have not heard new comment for our latest patch.
> So I send review request for it. Could you review it?
>
> webrev:
>   http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/
>
> Usage of new API:
>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-4347142/webrev.04/Test.java

 Yasumasa - I think the main issue here is to decide whether the API
 should be extended for encryption or not. If exposing it in the API make
 sense then I assume that alternative names to java.util.zip.ZipCryption
 needs to be explored.

 -Alan.

>

RFR 8151198: VarHandle factory-specific exceptions

[Paul Sandoz]

Hi

Please review:

  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151198-VH-factory-exceptions/webrev/
 


The patch tweaks the specification:

1) noting that factory methods may document additional undeclared exceptions 
that may be thrown by access methods e.g. IllegalSateException for VarHandles 
to ByteBuffer for misaligned access.

2) add the throwing of ClassCastException to each method, which i neglected to 
add when switching from invokeExact to invoke semantics.

Paul.

Re: RFR 8152645 VarHandle lookup access control tests

[Michael Haupt]

Hi Paul,

note this is a lower-case review. Thumbs up.

I like how the test lucidly documents the access rules, and would applaud an 
extended test that additionally covers module boundaries.

Just as a suggestion, how about using the fact that enum values are technically 
instances of subclasses of the enum and getting rid of the switches in 
FieldLookup.lookup/isAccessibleField by replacing the two with overridden 
methods in each of the enum elements? Switching over "this" just calls for 
polymorphism, and the default cases are dead code. Admittedly, it's a matter of 
style. :-)

Best,

Michael

> Am 07.04.2016 um 11:07 schrieb Paul Sandoz :
> 
> Hi,
> 
> Please review a test to verify access control of looking up fields using a 
> VarHandle:
> 
>  
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8152645-VH-access-control/webrev/
>  
> 
> 
> For completeness i also added tests for MH as i could not find any such 
> existing tests.
> 
> 
> Further follow on work might be to test lookup to fields across module 
> boundaries.
> 
> Paul.

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany

ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
  Oracle is committed to developing 
practices and products that help protect the environment

Re: RFR 8151706: Update VarHandle implementation to use @Stable arrays

[Michael Haupt]

Hi Paul,

note this is a lower-case review. Having looked at 8151705, thumbs up for this 
one as well - they go hand in hand and looking at one of them only doesn't feel 
right. :-)

Best,

Michael

> Am 08.04.2016 um 11:56 schrieb Paul Sandoz :
> 
> Hi,
> 
> Please review:
> 
>  
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151706-VH-form-table-stable/webrev/
>  
> 
> 
> Now that @Stable arrays are supported by C1 (thanks Vladimir!) we can switch 
> from the explicit use of MemberName fields in VarForm to a @Stable 
> MemberName[] array.
> 
> I also took the opportunity to simplify the linking from a VarHandle impl to 
> MemberName[] array, now that the implementation has settled down. This will 
> reduce initialization costs and memory churn.
> 
> 
> I held off making further improvements for now. For example, VarForm can 
> probably go away (also removing the dependency on ClassValue). A VarHandle 
> instance can directly hold a MemberName[] array whose source reference is 
> statically held on the associated VarHandle implementation. It should also be 
> possible to lazily create MemberName instances as required, rather than 
> aggressively doing so, which may further reduce initialization costs and 
> memory usage in common cases.
> 
> Paul.

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany

ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
  Oracle is committed to developing 
practices and products that help protect the environment

Re: RFR 8151705 VarHandle.AccessMode enum names should conform to code style

[Michael Haupt]

Hi Paul,

note this is a lower-case review. Thumbs up, with one remarks (no new webrev 
required IMO).

* VarHandle.java: static initialiser of AccessMode, comment:
  // Initial capacity of # values will is sufficient to avoid resizes
  -> remove "will"

Best,

Michael

> Am 07.04.2016 um 15:39 schrieb Paul Sandoz :
> 
> Hi,
> 
> Please review:
> 
>  
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151705-VH-AccessMode-names/webrev/
> 
> I was persuaded (in internal CCC review) to change the enum 
> VarHandle.AccessMode constant names to conform to the expected Java style.
> 
> Previously, for convenience, the names corresponded exactly with the VH 
> sig-poly method names.
> 
> Most of the patch is just refactoring.
> 
> I have added two new methods to translate to/from the sig-poly method name 
> domain:
> 
> 1175 /**
> 1176  * Returns the {@code VarHandle} signature-polymorphic method 
> name
> 1177  * associated with this {@code AccessMode} value
> 1178  *
> 1179  * @return the signature-polymorphic method name
> 1180  * @see #valueFromMethodName
> 1181  */
> 1182 public String methodName() {
> 1183 return methodName;
> 1184 }
> 1185
> 1186 /**
> 1187  * Returns the {@code AccessMode} value associated with the 
> specified
> 1188  * {@code VarHandle} signature-polymorphic method name.
> 1189  *
> 1190  * @param methodName the signature-polymorphic method name
> 1191  * @return the {@code AccessMode} value
> 1192  * @throws IllegalArgumentException if there is no {@code 
> AccessMode}
> 1193  * value associated with method name (indicating the 
> method
> 1194  * name does not correspond to a {@code VarHandle}
> 1195  * signature-polymorphic method name).
> 1196  * @see #methodName
> 1197  */
> 1198 public static AccessMode valueFromMethodName(String methodName) {
> 1199 AccessMode am = methodNameToAccessMode.get(methodName);
> 1200 if (am != null) return am;
> 1201 throw new IllegalArgumentException("No AccessMode value for 
> method name " + methodName);
> 1202 }
> 
> Paul.
> 

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany

ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
  Oracle is committed to developing 
practices and products that help protect the environment

Re: RFR 8151706: Update VarHandle implementation to use @Stable arrays

[Aleksey Shipilev]

On 04/08/2016 12:56 PM, Paul Sandoz wrote:
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151706-VH-form-table-stable/webrev/
> 
>
>  Now that @Stable arrays are supported by C1 (thanks Vladimir!) we
> can switch from the explicit use of MemberName fields in VarForm to a
> @Stable MemberName[] array.

Yes, glad to see that abomination gone.

I've checked C1 and C2 performance against our battery of VarHandle
microbenchmarks, and there were no regressions. So, +1.

Thanks,
-Aleksey

Re: RFR [9] 8153737: Unsupported Module

[Peter Levart]

On 04/08/2016 12:24 PM, Paul Sandoz wrote:

On 7 Apr 2016, at 19:14, Chris Hegarty  wrote:

Enough technical debt has been paid down that we can now create the new
JDK-specific module as proposed by JEP 260 [1], named jdk.unsupported.
This module will initially contain, and export, the sun.misc package,
and will eventually export the sun.reflect package too ( once it has
been sanitized ). The jdk.unsupported module will be present in full JRE
and JDK images.


And in the modular world it’s necessary to explicitly declare a requirement on 
idk.unsupported.


Will jdk.unsupported be "required public" by java.se? Will you have to 
explicitly -addmodule jdk.unsupported for class-path programs too?


Regards, Peter




http://cr.openjdk.java.net/~chegar/8153737/
https://bugs.openjdk.java.net/browse/JDK-8153737


+1

An important milestone.

Paul.

Re: RFR 8151705 VarHandle.AccessMode enum names should conform to code style

[Aleksey Shipilev]

On 04/07/2016 04:39 PM, Paul Sandoz wrote:
>   
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151705-VH-AccessMode-names/webrev/

Looks good. The performance is OK.

-Aleksey

Re: RFR [9] 8153737: Unsupported Module

[Chris Hegarty]

On 08/04/16 15:31, Peter Levart wrote:



On 04/08/2016 12:24 PM, Paul Sandoz wrote:

On 7 Apr 2016, at 19:14, Chris Hegarty  wrote:

Enough technical debt has been paid down that we can now create the new
JDK-specific module as proposed by JEP 260 [1], named jdk.unsupported.
This module will initially contain, and export, the sun.misc package,
and will eventually export the sun.reflect package too ( once it has
been sanitized ). The jdk.unsupported module will be present in full JRE
and JDK images.


And in the modular world it’s necessary to explicitly declare a
requirement on idk.unsupported.


Will jdk.unsupported be "required public" by java.se?


No.  Since jdk.unsupported will be in the JDK and JRE images,
and it exports sun.misc, then it will be available ( see below ).

>  Will you have to

explicitly -addmodule jdk.unsupported for class-path programs too?


Applications running on the class path will be able to access
sun.misc without any command line flags, since it is exported.

Modular applications, if they wish to use sun.misc, will have to
'require jdk.unsupported'.

-Chris.


Regards, Peter




http://cr.openjdk.java.net/~chegar/8153737/
https://bugs.openjdk.java.net/browse/JDK-8153737


+1

An important milestone.

Paul.

Re: RFR 8151198: VarHandle factory-specific exceptions

[Aleksey Shipilev]

On 04/08/2016 03:51 PM, Paul Sandoz wrote:
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151198-VH-factory-exceptions/webrev/
> 

I agree with updates.

But have more comments on C/C++ @apiNote-s. Would you like to address
them in follow-ups? Notably, getVolatile may be also marked analogous to
memory_order_seq_cst, and get/setOpaque may be marked analogous to C/C++
"volatile"-s. And CAS/CAE memory semantics can also be spelled out in
terms of std::memory_order.

Thanks,
-Aleksey

Re: JDK 9 proposal: allocating ByteBuffers on heterogeneous memory

[Dohrmann, Steve]

Hi Paul,

One point I meant to call out in the last message but did not...

Consistent with current ByteBuffers, we don't foresee public constructors on 
any ByteBuffer implementation classes themselves.  Public constructors are only 
proposed for the class that implements the Memory interface with its ByteBuffer 
factory method.

Thanks,
Steve

On Apr 7, 2016, at 3:03 PM, Dohrmann, Steve 
mailto:steve.dohrm...@intel.com>> wrote:

Hi Paul,

We would like to have an an API for Intel's 3D XPoint memory sooner than the 
JDK 10 timeframe and proposed this API because it seems simple enough to 
consider for JDK 9.  As you suggest, we will participate in the Panama 
discussions  in this area.  Any additional guidance you have would be 
appreciated.

Just to clarify, it is incidental that the proposed Memory interface has only 
one method.  We see the value of the interface as nominative; a new type that 
can be passed around to abstract various sources of ByteBuffer memory.

Regarding construction and allocation, our current Memory implementation 
allocates ByteBuffers by calling the NewDirectByteBuffer JNI function with a 
pointer to 3D XPoint memory allocated via a supporting native library.  The 
Linux libraries we have worked with are NVML (https://github.com/pmem/nvml/) 
and memkind (https://github.com/memkind/memkind).  We recently also became 
aware of the NVM-Direct library (https://github.com/oracle/NVM-Direct).  We 
currently don't need our own subclass and return the ByteBuffer returned by the 
JNI call.

Thanks,
Steve

On Apr 6, 2016, at 4:10 AM, Paul Sandoz 
mailto:paul.san...@oracle.com>> wrote:

Hi Steve,

My feeling is it’s too premature to introduce a general Memory (region) 
allocation interface at this moment. What is currently specified can be 
supported using:

 IntFunction

But i don’t wanna discourage you! this thread has raised some interesting 
points.

Project Panama is gonna take a swing at defining a more general notion of a 
memory region and the Arrays 2.0 work should support indexes greater than 
Integer.MAX_VALUE.

In this respect I think we should hold off doing anything premature for Java 9 
(feature freeze is getting closer), and i encourage you to participate on the 
Panama lists.


—

Here is some context some of which you probably know and some which you might 
not:

- ByteBuffer constructors are deliberately package scoped, as there are some 
intricate dependencies between the implementations and we want control over who 
can implement. Any new form of allocation will require changes here (package 
scoped or otherwise).

- current ByteBuffer implementations are either backed by a byte[] (managed or 
non-direct) or an off-heap allocated (direct) region. At the moment access to 
both those regions go through separate code paths, but they could be unified 
using the Unsafe double addressing mode, which should greatly simplify the 
implementations. I have started making some small steps towards that 
(JDK-8149469 and JDK-8151163). Even these small steps require careful analysis 
to evaluate performance across multiple platforms.

- VarHandles leverages the Unsafe double addressing mode to support enhanced 
atomic access to heap or direct ByteBuffers [1], thus the same code is used in 
both kinds of buffer.


—

I am not sure what plans you have for buffer implementations themselves.

How do you propose to allocate a ByteBuffer instance that covers a region of 3D 
XPoint memory?

Would it be similar to that of direct buffers, e.g. a variation of 
DirectByteBuffer, but with different factory constructor code to allocate the 
memory region within the XPoint memory system and assign the buffer base 
address to the start of that allocated region?

Thanks,
Paul.

[1] 
http://cr.openjdk.java.net/~psandoz/jdk9/varhandles/specdiff/java/lang/invoke/MethodHandles-report.html#method:byteBufferViewVarHandle(java.lang.Class,
 boolean)


On 6 Apr 2016, at 03:49, Dohrmann, Steve 
mailto:steve.dohrm...@intel.com>> wrote:

Re: JDK-8153111


Hi,

Below are responses to some of the points brought up in the discussion as well 
as is a little expansion of the reasoning that went into the proposed API.

One motivation we saw for doing anything beyond a concrete ByteBuffer class was 
application code (e.g. Cassandra) that allocates many off-heap ByteBuffers 
using ByteBuffer#allocateDirect.  We reasoned that if the allocation of 
ByteBuffers could be done using a common memory interface then only the code 
that provisioned instances of the the memory spaces would have to change in 
order to switch or mix memory types.

We did think of overloading the ByteBuffer#allocateDirect method with memory 
space info and avoid an allocation interface.  We ended up with a separate user 
called interface scheme because we imagined that extensions of the memory 
interface would enable new memory functionality that required new methods (e.g. 
memory transactions for 

Re: RFR [9] 8153737: Unsupported Module

[Alan Bateman]

On 08/04/2016 15:31, Peter Levart wrote:


Will jdk.unsupported be "required public" by java.se? 
No because jdk.* are JDK-specific and should never be required by 
standard modules.



Will you have to explicitly -addmodule jdk.unsupported for class-path 
programs too?
It exports an API and therefore will be treated as a root module, so no 
-addmods. If you develop your own module that uses sun.misc.Unsafe then 
it will of course need to declare its dependency on jdk.unsupported.


-Alan.

RFR: 8148568: LoggerFinder.getLogger and LoggerFinder.getLocalizedLogger should take a Module argument instead of a Class.

[Daniel Fuchs]

Hi,

Please find below a patch that slightly modifies
the JEP 264 initial implementation to take advantage
of the module system.

The change modifies the LoggerFinder abstract service
to take the Module of the caller class as parameter
instead of the caller class itself.
The caller class was used in the initial 9/dev implementation
because java.lang.reflect.Module was not yet available.

http://cr.openjdk.java.net/~dfuchs/jigsaw/webrev_8148568/webrev.01/

best regards,

-- daniel

Re: RFR: 8151901: test/tools/pack200/Pack200Test fails on verifying native unpacked JAR

[Kumar Srinivasan]

Thanks for the review, but your patches  appears to have
caused regressions, I will need to look at it this further,
when I get some more cycles.

Thanks
Kumar


Good.

Please change the comment:
s/null, no tuple change, force recomputation/null, drop ICs attribute, force CP 
recomputation/

Reordering BSMs and ICs should work.
The BSMs may need extra ICs, but ICs cannot depend on BSMs.
I wonder why we did the other order first?  I guess because that is what the 
spec. says.

Oh, there's a problem with the attribute insertion order logic; it's buggy that 
we unconditionally
insert a BSMs attr. and then delete it later.  (That goes wrong when change<0 
and we recompute
from scratch.)

Suggested amended patch enclosed.

--- John

On Mar 21, 2016, at 12:49 PM, Kumar Srinivasan  
wrote:

Hi John,

This patch contains fixes for two bugs:
8151901: test/tools/pack200/Pack200Test fails on verifying native unpacked JAR
8062335: Pack200 Java implementation differs from C version, outputs unused 
UTF8 for BootstrapMethods Note: The test case exhibits both of the above.

With this  the classes produced by java and the native implementation are 
congruent,
though the JDK image's classes exhibit these issues, as a precaution I have 
extracted the
minimal set of classes to reproduce the issues, into the golden jar.

The webrev is at:
http://cr.openjdk.java.net/~ksrini/8151901/webrev.00/

Thanks
Kumar




diff --git 
a/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java 
b/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java
--- a/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java
+++ b/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java
@@ -312,6 +312,12 @@
  void setBootstrapMethods(Collection bsms) {
  assert(bootstrapMethods == null);  // do not do this twice
  bootstrapMethods = new ArrayList<>(bsms);
+// Edit the attribute list, if necessary.
+Attribute a = getAttribute(attrBootstrapMethodsEmpty);
+if (bootstrapMethods != null && a == null)
+addAttribute(attrBootstrapMethodsEmpty.canonicalInstance());
+else if (bootstrapMethods == null && a != null)
+removeAttribute(a);
  }
  
  boolean hasInnerClasses() {

diff --git 
a/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java 
b/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java
--- a/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java
+++ b/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java
@@ -1193,16 +1193,25 @@
  cls.visitRefs(VRM_CLASSIC, cpRefs);
  
  ArrayList bsms = new ArrayList<>();

-/*
- * BootstrapMethod(BSMs) are added here before InnerClasses(ICs),
- * so as to ensure the order. Noting that the BSMs  may be
- * removed if they are not found in the CP, after the ICs expansion.
- */
-
cls.addAttribute(Package.attrBootstrapMethodsEmpty.canonicalInstance());
  
  // flesh out the local constant pool

  ConstantPool.completeReferencesIn(cpRefs, true, bsms);
  
+// Add the BSMs and references as required.

+if (!bsms.isEmpty()) {
+// Add the attirbute name to the CP (visitRefs would have wanted 
this).
+cpRefs.add(Package.getRefString("BootstrapMethods"));
+// The pack-spec requires that BootstrapMethods precedes the 
InnerClasses attribute.
+// So add BSMs now before running expandLocalICs.
+// But first delete any local InnerClasses attr.  (This is rare.)
+List localICs = cls.getInnerClasses();
+cls.setInnerClasses(null);
+Collections.sort(bsms);
+cls.setBootstrapMethods(bsms);
+// Re-add the ICs, so the attribute lists are in the required 
order.
+cls.setInnerClasses(localICs);
+}
+
  // Now that we know all our local class references,
  // compute the InnerClasses attribute.
  int changed = cls.expandLocalICs();
@@ -1221,16 +1230,6 @@
  ConstantPool.completeReferencesIn(cpRefs, true, bsms);
  }
  
-// remove the attr previously set, otherwise add the bsm and

-// references as required
-if (bsms.isEmpty()) {
-
cls.attributes.remove(Package.attrBootstrapMethodsEmpty.canonicalInstance());
-} else {
-cpRefs.add(Package.getRefString("BootstrapMethods"));
-Collections.sort(bsms);
-cls.setBootstrapMethods(bsms);
-}
-
  // construct a local constant pool
  int numDoubles = 0;
  for (Entry e : cpRefs) {

Re: RFR [9] 8153737: Unsupported Module

[Chris Hegarty]

On 07/04/16 19:57, Alan Bateman wrote:

On 07/04/2016 18:14, Chris Hegarty wrote:

Enough technical debt has been paid down that we can now create the new
JDK-specific module as proposed by JEP 260 [1], named jdk.unsupported.
This module will initially contain, and export, the sun.misc package,
and will eventually export the sun.reflect package too ( once it has
been sanitized ). The jdk.unsupported module will be present in full JRE
and JDK images.

http://cr.openjdk.java.net/~chegar/8153737/

It's great to get to this milestone.

I looked over the webrev and I don't see any issues.


Thanks for the Review.


I'm surprised by
the number of tests that had @modules java.base/sun.misc even though
they aren't using anything in sun.misc.


It was surprising to me too.


I see Phil has pushed JDK-8147544 to jdk9/client rather than jdk9/dev so
it means that the module-info.java for java.desktop will need to be
fixed up when it gets to jdk9/dev.


Right. I have my eye on that, and will fix it once in jdk9/dev.


I think we should bump the priority of all the issues that cause you to
add `requires jdk.unsupported`we should at least get all of them
resolved by FC as we shouldn't have any standard modules with this
dependence.


I bumped the priority, and targeted the issues to 9.

-Chris.

Re: RFR [9] 8153737: Unsupported Module

[Chris Hegarty]

On 08/04/16 03:52, Mandy Chung wrote:



On Apr 7, 2016, at 10:14 AM, Chris Hegarty  wrote:

Enough technical debt has been paid down that we can now create the new
JDK-specific module as proposed by JEP 260 [1], named jdk.unsupported.
This module will initially contain, and export, the sun.misc package,
and will eventually export the sun.reflect package too ( once it has
been sanitized ). The jdk.unsupported module will be present in full JRE
and JDK images.

http://cr.openjdk.java.net/~chegar/8153737/
https://bugs.openjdk.java.net/browse/JDK-8153737


I’m glad to see jdk.unsupported module be created.

jdeps tests are intended to test references to sun.misc and flagged as internal 
API. The jdeps test changes should be reverted.

It’s exported to allow existing applications to continue to run on JDK 9 while 
sun.misc and other critical internal APIs remains to be internal APIs.  jdeps 
-jdkinternals should flag them and get developers’ attention to prepare to 
migrate when the replacements are available.


I moved the tests from a directory named 'jdk.unsupported' to
unsupported', as other tests, in test/tools/jdeps/module, use
test/tools/jdeps as a test library, and the directory/test lib
name is conflicting with the module name. jtreg fails immediately.
The updates I made check that jdeps identifies both jdk.internal.misc
and sun.misc Unsafe as internal APIs.


Other than that, looks good.


Thanks for the Review.


There are so many tests that have @modules java.base sun/misc but unused.  It’s 
good that you clean that up.   I agree with Alan that JDK modules depending on 
jdk.unsupported should get the priority to get eliminated by FC.


I bumped the priority, and target the issues to 9.

-Chris.

Re: RFR [9] 8153737: Unsupported Module

[Mandy Chung]

> On Apr 8, 2016, at 8:35 AM, Chris Hegarty  wrote:
> 
> 
> I moved the tests from a directory named 'jdk.unsupported' to
> unsupported', as other tests, in test/tools/jdeps/module, use
> test/tools/jdeps as a test library, and the directory/test lib
> name is conflicting with the module name. jtreg fails immediately.
> The updates I made check that jdeps identifies both jdk.internal.misc
> and sun.misc Unsafe as internal APIs.

Thanks for explaining why you did the rename.  Your change now makes sense to 
me.  I may reorganize the jdeps tests to avoid the module name you ran into in 
the future.

All looks good.

Mandy

Re: RFR 8151198: VarHandle factory-specific exceptions

[Martin Buchholz]

On Fri, Apr 8, 2016 at 7:40 AM, Aleksey Shipilev
 wrote:
> On 04/08/2016 03:51 PM, Paul Sandoz wrote:
>> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151198-VH-factory-exceptions/webrev/
>> 
>
> I agree with updates.
>
> But have more comments on C/C++ @apiNote-s. Would you like to address
> them in follow-ups? Notably, getVolatile may be also marked analogous to
> memory_order_seq_cst, and get/setOpaque may be marked analogous to C/C++
> "volatile"-s. And CAS/CAE memory semantics can also be spelled out in
> terms of std::memory_order.

I agree with Aleksey.

Re: RFR 8151198: VarHandle factory-specific exceptions

[Paul Sandoz]

> On 8 Apr 2016, at 17:59, Martin Buchholz  wrote:
> 
> On Fri, Apr 8, 2016 at 7:40 AM, Aleksey Shipilev
>  wrote:
>> On 04/08/2016 03:51 PM, Paul Sandoz wrote:
>>> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8151198-VH-factory-exceptions/webrev/
>>> 
>> 
>> I agree with updates.
>> 
>> But have more comments on C/C++ @apiNote-s. Would you like to address
>> them in follow-ups? Notably, getVolatile may be also marked analogous to
>> memory_order_seq_cst, and get/setOpaque may be marked analogous to C/C++
>> "volatile"-s. And CAS/CAE memory semantics can also be spelled out in
>> terms of std::memory_order.
> 
> I agree with Aleksey.


No objections, i will follow up using 
https://bugs.openjdk.java.net/browse/JDK-8153875 


Thanks,
Paul.

Re: PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries

[Anthony Vanelverdinghe]

Hi Yuji, Sherman et al.

PKWARE's "Strong Encryption Specification" [1] indeed warns about 
patents, but how/why does this affect Winzip's AE-1 and AE-2 [2]?


I don't mind if decryption support is added for the "Traditional 
Encryption". However, I believe it would be wrong to introduce 
encryption support for a known-to-be-broken encryption method in the 
JDK. Using the argument of "it's good enough for our case", I could also 
argue that Base64 qualifies as an encryption method, or that SSLv2 is an 
appropriate choice to secure network connections.


However, if this is to be added, it should be supported by the zipfs 
FileSystemProvider as well. The passphrase handler could be passed by a 
property in the environment, e.g. Function> which 
would provide the passphrase for a given path, or Optional.empty() if 
the Path is unencrypted.


Kind regards, Anthony

[1] https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
[2] http://www.winzip.com/win/en/aes_info.htm

On 8/04/2016 13:04, KUBOTA Yuji wrote:

Hi Sherman and all,

Thank you for all comments about this proposal!

Thank Tomoyuki and Stephen for sharing your needs.

Thank Bernd for sharing your information. However, I am
afraid that AE1 and AE2 may have a risk of patent issue,
while "Traditional PKWare encryption" is explicitly free
from the risk according to the PKWare documents.
Therefore, I think this proposed implementation to support
legacy zip specification is meaningful as Tomoyuki said.

We got some positive opinions / needs. I am glad if you
consider the proposal acceptable for JDK.
If this proposed implementation is acceptable, I and
Yasumasa will improve it by corresponding to comments
which given by Sherman and Stephen.

Thanks!
Yuji

2016-03-29 6:41 GMT+09:00 Xueming Shen :

Yasumasa

It's a tricky call. To be honest, as I said at the very beginning, I'm not
sure whether
or not it's a good idea and worth the efffort to push this into the j.u.zip
package to
support the "traditional PKWare encryption", which is known to be "seriously
flawed,
and in particular is vulnerable to known-plaintext attacks" (from wiki),
while I fully
understood it is not a concern in your "problem-free" use scenario. Just
wonder
if there is anyone else on the list that has/had the need for such
encryption feature
in the past. It would be preferred to have more input (agree, disagree) to
make the
final decision.

Here are some comments regarding the proposed implementation,

(1) The changes in native code are probably no longer needed. The native
path is
  left there for vm directly access.
(2) I'm concerned about the footprint increase for the ZipEntry class (the
proposed
  change is adding 3 more fields into the entry class). Given this is
something rarely
  needed/used, and we might have hundreds and thousands of zip/jar
entries during
  startup/runtime, it might be desired to avoid adding these fields into
ZipEntry class.
  A possible alternative to have a dedicated entry class extended from
the ZipEntry
  to hold those extra fields and methods, EncryptionZipEntry for example.
So the
  proposed encryption support code will not have any impact to the
existing use of
  ZipInput/OutputStream/File.
(3) I'm also not comfortable to add the "encryption engine" logic into the
in/deflater
  stream classes, while it might be convenient to do so from
implementation point
  of view. Given the encryption is something between the raw bites in the
zip file
  and the in/deflating layer, it might need an extra layer there, though
I'm not sure
  if it's easy to do so.

The webrev below is what I think might be better for the ZipFile and
ZipOutputStream
to have an extra layer in between to handle the encryption. Not try to test
if it really
works, just throw in the idea for discussion.

http://cr.openjdk.java.net/~sherman/zencrypt/webrev/

No, I did not try the ZIS, the "pushback" stream there might be a little
tricky:-)

-Sherman


On 03/28/2016 02:34 AM, Yasumasa Suenaga wrote:

PING: Could you review it?
We want to move forward this enhancement.

Thanks,

Yasumasa
2016/03/19 22:01 "Yasumasa Suenaga":


Hi Alan,


I think the main issue here is to decide whether the API
should be extended for encryption or not.

We've discussed on the premise that we add the API for supporting ZIP
encryption.
In this context, Sherman tried to implement AES encryption extending
current API. [1]

IMHO, ZIP encryption should be implemented as extention of current ZIP
API
because encryption/decryption will process for each ZIP entries.

According to Developers' Guide, guideline for adding new API is TBD. [2]
What should I do next?


Thanks,

Yasumsa


[1]

http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/037903.html
[2] http://openjdk.java.net/guide/changePlanning.html#api


On 2016/03/19 0:25, Alan Bateman wrote:

On 18/03/2016 15:02, Yasumasa Suenaga wrote:

Hi all,

We (including me and Yuji Kubota (ykubota: OpenJDK jdk9 

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

[Steve Drach]

>> I’ve put up a new webrev that addresses the issue of having spaces before 
>> (and after) the value “true” in the Multi-Release attribute.
>> 
> 
> Is some or all of that really necessary? since the we can specify domain of 
> values.

I think it is.  The spec states that one can have an arbitrary amount of 
leading/trailing spaces in the value part of the attribute.  Apparently 
attribute values could also have “hidden” characters such as vertical tab or 
nak for example.  I wish the spec was tighter here.  I’m merely stating the the 
domain can be
“ *TRUE *” for upper/lower case letters.

> 
> For example, the Sealed attribute can take on a value of “true” or “false” 
> (case ignored), but there is no white space trimming.

Well then “Sealed:   true” won’t work, but the spec says it should work.

I am fine without doing this nonsense, but I think we need to change the spec 
to make it correct.  We could change the definition of "otherchar” to something 
like this

otherchar:=ALPHA EXTALPHANUM*
ALPHA:=[A-Z | a-z | _]
EXTALPHANUM:=[ALPHA | 0-9 | SPACE]

Even this will still allow trailing spaces, so after matching TRUE we’d need to 
make sure the next char is SPACE, \r, or \n.

Other ideas?

> 
> Paul.
> 
>>> Hi,
>>> Please review this simple fix to require that the jar manifest 
>>> Multi-Release attribute has a value of “true" in order to be effective, 
>>> i.e. to assert the jar is a multi-release jar.
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8153213 
>>> 
>>> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>>> 
>>> Thanks
>>> Steve
> 

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

[Paul Sandoz]

> On 8 Apr 2016, at 18:54, Steve Drach  wrote:
> 
>>> I’ve put up a new webrev that addresses the issue of having spaces before 
>>> (and after) the value “true” in the Multi-Release attribute.
>>> 
>> 
>> Is some or all of that really necessary? since the we can specify domain of 
>> values.
> 
> I think it is.  The spec states that one can have an arbitrary amount of 
> leading/trailing spaces in the value part of the attribute.  Apparently 
> attribute values could also have “hidden” characters such as vertical tab or 
> nak for example.  I wish the spec was tighter here.  I’m merely stating the 
> the domain can be
> “ *TRUE *” for upper/lower case letters.
> 

AFAICT the so called “spec" says nothing about trimming white space from values 
and although not explicitly called out the actual value has to be what 
constitutes the character sequence for the “otherchar" definition (otherwise 
the continuation space and newline characters would also be part of the actual 
value and that makes no sense).

So it seems you may have quite a bit of wiggle room here :-) and it’s up to 
each attribute definition to state what constitutes its domain of possible 
valid values based on “otherchar”.


>> 
>> For example, the Sealed attribute can take on a value of “true” or “false” 
>> (case ignored), but there is no white space trimming.
> 
> Well then “Sealed:   true” won’t work, but the spec says it should work.
> 

Where does the “spec” state that it should?

The value is literally the string “ true”, at least in one interpretation of 
the “spec” :-)

I suspect it’s ok to go with finding “Multi-Release: true” (lower/upper case) 
as in your first patch, then check if it is terminated with a newline (and 
ignore the continuation case)

Paul.


> I am fine without doing this nonsense, but I think we need to change the spec 
> to make it correct.  We could change the definition of "otherchar” to 
> something like this
> 
> otherchar:=ALPHA EXTALPHANUM*
> ALPHA:=[A-Z | a-z | _]
> EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
> 
> Even this will still allow trailing spaces, so after matching TRUE we’d need 
> to make sure the next char is SPACE, \r, or \n.
> 
> Other ideas?
> 
>> 
>> Paul.
>> 
 Hi,
 Please review this simple fix to require that the jar manifest 
 Multi-Release attribute has a value of “true" in order to be effective, 
 i.e. to assert the jar is a multi-release jar.
 issue: https://bugs.openjdk.java.net/browse/JDK-8153213 
 
 webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
 
 Thanks
 Steve
>> 
> 

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

[Steve Drach]

 I’ve put up a new webrev that addresses the issue of having spaces before 
 (and after) the value “true” in the Multi-Release attribute.
 
>>> 
>>> Is some or all of that really necessary? since the we can specify domain of 
>>> values.
>> 
>> I think it is.  The spec states that one can have an arbitrary amount of 
>> leading/trailing spaces in the value part of the attribute.  Apparently 
>> attribute values could also have “hidden” characters such as vertical tab or 
>> nak for example.  I wish the spec was tighter here.  I’m merely stating the 
>> the domain can be
>> “ *TRUE *” for upper/lower case letters.
>> 
> 
> AFAICT the so called “spec" says nothing about trimming white space from 
> values and although not explicitly called out the actual value has to be what 
> constitutes the character sequence for the “otherchar" definition (otherwise 
> the continuation space and newline characters would also be part of the 
> actual value and that makes no sense).

No it doesn’t say you can trim white space, but if one doesn’t, then do we 
accept “true”, “ true”, “  true”, etc.?

> 
> So it seems you may have quite a bit of wiggle room here :-) and it’s up to 
> each attribute definition to state what constitutes its domain of possible 
> valid values based on “other char”.

So, we can say *otherchar is upper/lower case “true” only?

> 
> 
>>> 
>>> For example, the Sealed attribute can take on a value of “true” or “false” 
>>> (case ignored), but there is no white space trimming.
>> 
>> Well then “Sealed:   true” won’t work, but the spec says it should work.
>> 
> 
> Where does the “spec” state that it should?

It really comes down to the interpretation of otherchar.  If it can be 
interpreted on an attribute by attribute basis then it’s not a problem.

> 
> The value is literally the string “ true”, at least in one interpretation of 
> the “spec” :-)

I’m fine with that.  And we really should do something about the spec, it’s too 
loose and ambiguous.

> 
> I suspect it’s ok to go with finding “Multi-Release: true” (lower/upper case) 
> as in your first patch, then check if it is terminated with a newline (and 
> ignore the continuation case)

Okay.  I’d like Claes to weigh in because he’s the one who brought it up.  He’s 
traveling today, so I don’t expect to hear from him soon.

> 
> Paul.
> 
> 
>> I am fine without doing this nonsense, but I think we need to change the 
>> spec to make it correct.  We could change the definition of "otherchar” to 
>> something like this
>> 
>> otherchar:=ALPHA EXTALPHANUM*
>> ALPHA:=[A-Z | a-z | _]
>> EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
>> 
>> Even this will still allow trailing spaces, so after matching TRUE we’d need 
>> to make sure the next char is SPACE, \r, or \n.
>> 
>> Other ideas?
>> 
>>> 
>>> Paul.
>>> 
> Hi,
> Please review this simple fix to require that the jar manifest 
> Multi-Release attribute has a value of “true" in order to be effective, 
> i.e. to assert the jar is a multi-release jar.
> issue: https://bugs.openjdk.java.net/browse/JDK-8153213 
> 
> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
> 
> Thanks
> Steve
>>> 
>> 
> 

Re: JDK 9 RFR of JDK-8153722: Mark java/nio/channels/Selector/SelectAndClose.java as intermittently failing

[Brian Burkhalter]

[Looping in nio-dev …]

+1 (but you still need Reviewer approval).

Brian

On Apr 8, 2016, at 3:40 AM, Amy Lu  wrote:

> java/nio/channels/Selector/SelectAndClose.java
> 
> This test is known to fail intermittently (JDK-8152814). This patch is to 
> mark the test accordingly with keyword 'intermittent'.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8153722
> webrev: http://cr.openjdk.java.net/~amlu/8153722/webrev.00/

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

[Claes Redestad]

On 04/08/2016 07:54 PM, Steve Drach wrote:

I’ve put up a new webrev that addresses the issue of having spaces before (and 
after) the value “true” in the Multi-Release attribute.


Is some or all of that really necessary? since the we can specify domain of 
values.

I think it is.  The spec states that one can have an arbitrary amount of 
leading/trailing spaces in the value part of the attribute.  Apparently 
attribute values could also have “hidden” characters such as vertical tab or 
nak for example.  I wish the spec was tighter here.  I’m merely stating the the 
domain can be
“ *TRUE *” for upper/lower case letters.


AFAICT the so called “spec" says nothing about trimming white space from values and 
although not explicitly called out the actual value has to be what constitutes the 
character sequence for the “otherchar" definition (otherwise the continuation space 
and newline characters would also be part of the actual value and that makes no sense).

No it doesn’t say you can trim white space, but if one doesn’t, then do we 
accept “true”, “ true”, “  true”, etc.?


So it seems you may have quite a bit of wiggle room here :-) and it’s up to 
each attribute definition to state what constitutes its domain of possible 
valid values based on “other char”.

So, we can say *otherchar is upper/lower case “true” only?




For example, the Sealed attribute can take on a value of “true” or “false” 
(case ignored), but there is no white space trimming.

Well then “Sealed:   true” won’t work, but the spec says it should work.


Where does the “spec” state that it should?

It really comes down to the interpretation of otherchar.  If it can be 
interpreted on an attribute by attribute basis then it’s not a problem.


The value is literally the string “ true”, at least in one interpretation of 
the “spec” :-)

I’m fine with that.  And we really should do something about the spec, it’s too 
loose and ambiguous.


I suspect it’s ok to go with finding “Multi-Release: true” (lower/upper case) 
as in your first patch, then check if it is terminated with a newline (and 
ignore the continuation case)

Okay.  I’d like Claes to weigh in because he’s the one who brought it up.  He’s 
traveling today, so I don’t expect to hear from him soon.


Yeah, I'm guilty of raising the silly question of whether or not 
accepting untrimmed true is OK according to spec.


With a generous interpretation and the presence of prior art (Sealed) I 
think it's OK to go back to the first version of the patch (with the 
addition to check that Multi-Release: true is followed by a LF, CR or 
the end of the manifest) and add a similar note to the spec/notes that 
Multi-Release has the same value restrictions as Sealed.


Perhaps both cases should be clarified in the notes to say that 
leading/trailing whitespace is not accepted, since that isn't directly 
obvious to a casual reader.


Thanks!

/Claes




Paul.



I am fine without doing this nonsense, but I think we need to change the spec to 
make it correct.  We could change the definition of "otherchar” to something 
like this

otherchar:=ALPHA EXTALPHANUM*
ALPHA:=[A-Z | a-z | _]
EXTALPHANUM:=[ALPHA | 0-9 | SPACE]

Even this will still allow trailing spaces, so after matching TRUE we’d need to 
make sure the next char is SPACE, \r, or \n.

Other ideas?


Paul.


Hi,
Please review this simple fix to require that the jar manifest Multi-Release 
attribute has a value of “true" in order to be effective, i.e. to assert the 
jar is a multi-release jar.
issue: https://bugs.openjdk.java.net/browse/JDK-8153213 

webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 

Thanks
Steve

Re: JDK 9 RFR of JDK-8153722: Mark java/nio/channels/Selector/SelectAndClose.java as intermittently failing

[joe darcy]

Looks fine Amy; thanks,

-Joe

On 4/8/2016 11:12 AM, Brian Burkhalter wrote:

[Looping in nio-dev …]

+1 (but you still need Reviewer approval).

Brian

On Apr 8, 2016, at 3:40 AM, Amy Lu  wrote:


java/nio/channels/Selector/SelectAndClose.java

This test is known to fail intermittently (JDK-8152814). This patch is to mark 
the test accordingly with keyword 'intermittent'.

bug: https://bugs.openjdk.java.net/browse/JDK-8153722
webrev: http://cr.openjdk.java.net/~amlu/8153722/webrev.00/

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

[Steve Drach]

Okay, I’ll prepare a new webrev.  I think all we need to check for after “true” 
is \r or \n.  If the manifest just ends without at least one of those, it’s not 
a legal manifest.

> On Apr 8, 2016, at 11:25 AM, Claes Redestad  wrote:
> 
> On 04/08/2016 07:54 PM, Steve Drach wrote:
>> I’ve put up a new webrev that addresses the issue of having spaces 
>> before (and after) the value “true” in the Multi-Release attribute.
>> 
> Is some or all of that really necessary? since the we can specify domain 
> of values.
 I think it is.  The spec states that one can have an arbitrary amount of 
 leading/trailing spaces in the value part of the attribute.  Apparently 
 attribute values could also have “hidden” characters such as vertical tab 
 or nak for example.  I wish the spec was tighter here.  I’m merely stating 
 the the domain can be
 “ *TRUE *” for upper/lower case letters.
 
>>> AFAICT the so called “spec" says nothing about trimming white space from 
>>> values and although not explicitly called out the actual value has to be 
>>> what constitutes the character sequence for the “otherchar" definition 
>>> (otherwise the continuation space and newline characters would also be part 
>>> of the actual value and that makes no sense).
>> No it doesn’t say you can trim white space, but if one doesn’t, then do we 
>> accept “true”, “ true”, “  true”, etc.?
>> 
>>> So it seems you may have quite a bit of wiggle room here :-) and it’s up to 
>>> each attribute definition to state what constitutes its domain of possible 
>>> valid values based on “other char”.
>> So, we can say *otherchar is upper/lower case “true” only?
>> 
>>> 
> For example, the Sealed attribute can take on a value of “true” or 
> “false” (case ignored), but there is no white space trimming.
 Well then “Sealed:   true” won’t work, but the spec says it should work.
 
>>> Where does the “spec” state that it should?
>> It really comes down to the interpretation of otherchar.  If it can be 
>> interpreted on an attribute by attribute basis then it’s not a problem.
>> 
>>> The value is literally the string “ true”, at least in one interpretation 
>>> of the “spec” :-)
>> I’m fine with that.  And we really should do something about the spec, it’s 
>> too loose and ambiguous.
>> 
>>> I suspect it’s ok to go with finding “Multi-Release: true” (lower/upper 
>>> case) as in your first patch, then check if it is terminated with a newline 
>>> (and ignore the continuation case)
>> Okay.  I’d like Claes to weigh in because he’s the one who brought it up.  
>> He’s traveling today, so I don’t expect to hear from him soon.
> 
> Yeah, I'm guilty of raising the silly question of whether or not accepting 
> untrimmed true is OK according to spec.
> 
> With a generous interpretation and the presence of prior art (Sealed) I think 
> it's OK to go back to the first version of the patch (with the addition to 
> check that Multi-Release: true is followed by a LF, CR or the end of the 
> manifest) and add a similar note to the spec/notes that Multi-Release has the 
> same value restrictions as Sealed.
> 
> Perhaps both cases should be clarified in the notes to say that 
> leading/trailing whitespace is not accepted, since that isn't directly 
> obvious to a casual reader.
> 
> Thanks!
> 
> /Claes
> 
>> 
>>> Paul.
>>> 
>>> 
 I am fine without doing this nonsense, but I think we need to change the 
 spec to make it correct.  We could change the definition of "otherchar” to 
 something like this
 
 otherchar:=ALPHA EXTALPHANUM*
 ALPHA:=[A-Z | a-z | _]
 EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
 
 Even this will still allow trailing spaces, so after matching TRUE we’d 
 need to make sure the next char is SPACE, \r, or \n.
 
 Other ideas?
 
> Paul.
> 
>>> Hi,
>>> Please review this simple fix to require that the jar manifest 
>>> Multi-Release attribute has a value of “true" in order to be effective, 
>>> i.e. to assert the jar is a multi-release jar.
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8153213 
>>> 
>>> webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
>>> 
>>> Thanks
>>> Steve

Review request 8153665: URLClassLoader.definePackage() not throwing expected IAE

[Mandy Chung]

The spec of java.lang.Package and ClassLoader::definePackage have been changed 
in jdk-9+111 such that Package objects are defined per class loader and no 
longer inspect what packages are defined in the class loader hierarchy. 

The spec of URLClassLoader::definePackage(String name, Manifest man, URL url) 
should also be changed accordingly.

Webrev at:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153665/webrev.00/

Mandy

Re: RFR 8151198: VarHandle factory-specific exceptions

[Aleksey Shipilev]

On 04/08/2016 09:39 PM, Hans Boehm wrote:
> I didn't previously have the impression that get/setOpaque was analogous
> to just C/C++ volatile.  C volatile officially has nothing to do with
> threads and does not prevent undefined behavior for data races.  It does
> not in general guarantee single-copy atomicity.  

Yes, Opaque does not imply memory ordering either.

> My prior impression was that Opaque was intended to be similar to a C++
> memory_order_relaxed access to a variable that is declared as both
> atomic and volatile, with the unordered interpretation of C++
> "volatile".  

Yes, that seems to be the intended semantics; sorry for the confusion.
Let's have this discussion when Paul submits the follow-up fix for
https://bugs.openjdk.java.net/browse/JDK-8153875; I linked your reply
there. Some cross-check against what is implemented in C1 and C2 would
be required at that point.

Thanks,
-Aleksey

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

[Steve Drach]

The new webrev is http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 

and the issue is https://bugs.openjdk.java.net/browse/JDK-8153213 


Now the value “true” followed by either ‘\r’ or ‘\n’ is the only acceptable 
value.

> On Apr 8, 2016, at 11:34 AM, Steve Drach  wrote:
> 
> Okay, I’ll prepare a new webrev.  I think all we need to check for after 
> “true” is \r or \n.  If the manifest just ends without at least one of those, 
> it’s not a legal manifest.
> 
>> On Apr 8, 2016, at 11:25 AM, Claes Redestad  
>> wrote:
>> 
>> On 04/08/2016 07:54 PM, Steve Drach wrote:
>>> I’ve put up a new webrev that addresses the issue of having spaces 
>>> before (and after) the value “true” in the Multi-Release attribute.
>>> 
>> Is some or all of that really necessary? since the we can specify domain 
>> of values.
> I think it is.  The spec states that one can have an arbitrary amount of 
> leading/trailing spaces in the value part of the attribute.  Apparently 
> attribute values could also have “hidden” characters such as vertical tab 
> or nak for example.  I wish the spec was tighter here.  I’m merely 
> stating the the domain can be
> “ *TRUE *” for upper/lower case letters.
> 
 AFAICT the so called “spec" says nothing about trimming white space from 
 values and although not explicitly called out the actual value has to be 
 what constitutes the character sequence for the “otherchar" definition 
 (otherwise the continuation space and newline characters would also be 
 part of the actual value and that makes no sense).
>>> No it doesn’t say you can trim white space, but if one doesn’t, then do we 
>>> accept “true”, “ true”, “  true”, etc.?
>>> 
 So it seems you may have quite a bit of wiggle room here :-) and it’s up 
 to each attribute definition to state what constitutes its domain of 
 possible valid values based on “other char”.
>>> So, we can say *otherchar is upper/lower case “true” only?
>>> 
 
>> For example, the Sealed attribute can take on a value of “true” or 
>> “false” (case ignored), but there is no white space trimming.
> Well then “Sealed:   true” won’t work, but the spec says it should work.
> 
 Where does the “spec” state that it should?
>>> It really comes down to the interpretation of otherchar.  If it can be 
>>> interpreted on an attribute by attribute basis then it’s not a problem.
>>> 
 The value is literally the string “ true”, at least in one interpretation 
 of the “spec” :-)
>>> I’m fine with that.  And we really should do something about the spec, it’s 
>>> too loose and ambiguous.
>>> 
 I suspect it’s ok to go with finding “Multi-Release: true” (lower/upper 
 case) as in your first patch, then check if it is terminated with a 
 newline (and ignore the continuation case)
>>> Okay.  I’d like Claes to weigh in because he’s the one who brought it up.  
>>> He’s traveling today, so I don’t expect to hear from him soon.
>> 
>> Yeah, I'm guilty of raising the silly question of whether or not accepting 
>> untrimmed true is OK according to spec.
>> 
>> With a generous interpretation and the presence of prior art (Sealed) I 
>> think it's OK to go back to the first version of the patch (with the 
>> addition to check that Multi-Release: true is followed by a LF, CR or the 
>> end of the manifest) and add a similar note to the spec/notes that 
>> Multi-Release has the same value restrictions as Sealed.
>> 
>> Perhaps both cases should be clarified in the notes to say that 
>> leading/trailing whitespace is not accepted, since that isn't directly 
>> obvious to a casual reader.
>> 
>> Thanks!
>> 
>> /Claes
>> 
>>> 
 Paul.
 
 
> I am fine without doing this nonsense, but I think we need to change the 
> spec to make it correct.  We could change the definition of "otherchar” 
> to something like this
> 
> otherchar:=ALPHA EXTALPHANUM*
> ALPHA:=[A-Z | a-z | _]
> EXTALPHANUM:=[ALPHA | 0-9 | SPACE]
> 
> Even this will still allow trailing spaces, so after matching TRUE we’d 
> need to make sure the next char is SPACE, \r, or \n.
> 
> Other ideas?
> 
>> Paul.
>> 
 Hi,
 Please review this simple fix to require that the jar manifest 
 Multi-Release attribute has a value of “true" in order to be 
 effective, i.e. to assert the jar is a multi-release jar.
 issue: https://bugs.openjdk.java.net/browse/JDK-8153213 
 
 webrev: http://cr.openjdk.java.net/~sdrach/8153213/webrev/index.html 
 
 Thanks
 Steve
> 

Re: RFR: JDK-8152115 (proxy) Examine performance of dynamic proxy creation

[Mandy Chung]

> On Apr 8, 2016, at 3:41 AM, Peter Levart  wrote:
> 
> Hi,
> 
> With Mandy we have prepared the following patch to restore performance of 
> java.lanf.reflect.Proxy class caching that has regressed with introduction of 
> additional checks that have to be performed due to modules:
> 
> http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.caching/webrev.05/
> 

Thanks for taking this on and further improving the performance. With this 
patch, existing code could move to newProxyInstance and remove any code to 
cache Proxy classes for performance reason.

The change looks quite good.  Minor comments below.

I like ClassLoaderValue idea that per-class loader proxy classes and dynamic 
modules are cached there that allows them to be GC’ed together.  
ClassLoaderValue might be useful for some other area and when those are 
identified, we could consider moving it to jdk.internal.

The javadoc of ClassLoaderValue has references to root-ClassLoaderValue and 
sub-ClassLoaderValue.  Sub is an inner class in AbstractClassLoaderValue.  
Should it be be ClassLoaderValue.Sub?  They are minor points as this is for 
proxy use only.  Something we could look into when we want to move it out from 
package scope.  

AbstractClassLoaderValue.java
 263  ? BootLoader.getClassLoaderValueMap()
 264  : JLA.createOrGetClassLoaderValueMap(cl)

Nit: I generally like ?-ternary indent to the right below the test.

Proxy.java

 376 private static Constructor getProxyConstructor(Class caller, // 
null if no SecurityManager
 377   ClassLoader loader,
 378   Class... 
interfaces)
 379 throws IllegalArgumentException
 380 {

IAE is an unchecked exception that does not need to be declared.  I realized 
it’s pre-existing code.  While you are on it, it can be taken out.  

The informal javadoc for getProxyConstructor would be helpful.  So the comment 
next to the caller parameter would be moved to the javadoc.

 384 if (caller != null) {
 385 checkProxyAccess(caller, loader, intf);
 386 }

The conditional check (caller != null) is not needed since checkProxyAccess and 
checkNewProxyPermission are nop if security manager is not present.  Same also 
applies to line 986.

 532 return Boolean.TRUE.equals(
 533 reverseProxyCache.sub(c).get(c.getClassLoader()));

Maybe Objects::equals that checks == first.

Driver.java
  28  * @bug 8152115
  29  * @summary functional and concurrency test for ClassLoaderValue

The convention we use to move @bug and @summary after @test.  It’d be good to 
move them up.

> 
> @Mandy
> 
> I haven't yet removed the superfluous checking and providing access from 
> dynamic module to the modules/packages of transitive closure of interfaces 
> implemented by proxy class. I think this should be done in a separate issue 
> so that this change doesn't change the semantics of implementation.
> 

I agree better to separate this as a different issue.  Perhaps I can take that 
one since I’d like to do more testing before pushing it.

Mandy

Re: Review request 8153665: URLClassLoader.definePackage() not throwing expected IAE

[Alan Bateman]

On 08/04/2016 19:48, Mandy Chung wrote:

The spec of java.lang.Package and ClassLoader::definePackage have been changed 
in jdk-9+111 such that Package objects are defined per class loader and no 
longer inspect what packages are defined in the class loader hierarchy.

The spec of URLClassLoader::definePackage(String name, Manifest man, URL url) 
should also be changed accordingly.

Webrev at:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153665/webrev.00/

This looks fine. For the SplitPackage test then you could use @library 
to use CompilerUtils and drop the compile method.


-Alan

Re: RFR: 8148568: LoggerFinder.getLogger and LoggerFinder.getLocalizedLogger should take a Module argument instead of a Class.

[Mandy Chung]

> On Apr 8, 2016, at 7:52 AM, Daniel Fuchs  wrote:
> 
> Hi,
> 
> Please find below a patch that slightly modifies
> the JEP 264 initial implementation to take advantage
> of the module system.
> 
> The change modifies the LoggerFinder abstract service
> to take the Module of the caller class as parameter
> instead of the caller class itself.
> The caller class was used in the initial 9/dev implementation
> because java.lang.reflect.Module was not yet available.
> 
> http://cr.openjdk.java.net/~dfuchs/jigsaw/webrev_8148568/webrev.01/

Using the Module as the caller granularity is reasonable here to find the 
resource bundle.

DefaultLoggerFinder.java
 135 static boolean isSystem(Module m) {
 136 ClassLoader cl = AccessController.doPrivileged(new 
PrivilegedAction() {

This isSystem method should check if the class loader is platform class loader 
(ClassLoader::getPlatformClassLoader) or its ancestor. There are several copies 
of this method. Better to have one single copy of this method shared for other 
classes to use?

Nit: you can use the diamond operation PrivilegedAction<>.

LazyLoggers.java.
line 294 return new LazyLoggerAccessor(name, factories, module);

I spot several long lines in LogManager.java, Logger.java, System.java and 
tests, maybe other files.  It’d be good if you can wrap the lines to help 
side-by-side review.

Other that that, looks good.

Mandy

RFR JDK 9 (JAXP) 8151162: Public entries not searched when prefer='system'

[huizhe wang]

Hi,

Please review a fix for the issue that caused public entries not being 
searched when prefer='system'. When prefer='system', the Catalog 
standard only required that public entries were not searched if there is 
a system identifier.


I added tests to reflect a combination of the settings of prefer, 
identifier(s), and entry types (public and/or system). The test 
"matchWithPrefer" covered the three test cases in the JCK mentioned in 
the bug report plus 9 other scenarios, while "resolveWithPrefer" covered 
18 resolution scenarios by a CatalogResolver.


JBS: https://bugs.openjdk.java.net/browse/JDK-8151162
Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8151162/webrev/

Thanks,
Joe

Re: Review request 8153665: URLClassLoader.definePackage() not throwing expected IAE

[Mandy Chung]

> On Apr 8, 2016, at 12:54 PM, Alan Bateman  wrote:
> 
> On 08/04/2016 19:48, Mandy Chung wrote:
>> The spec of java.lang.Package and ClassLoader::definePackage have been 
>> changed in jdk-9+111 such that Package objects are defined per class loader 
>> and no longer inspect what packages are defined in the class loader 
>> hierarchy.
>> 
>> The spec of URLClassLoader::definePackage(String name, Manifest man, URL 
>> url) should also be changed accordingly.
>> 
>> Webrev at:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153665/webrev.00/
>> 
> This looks fine. For the SplitPackage test then you could use @library to use 
> CompilerUtils and drop the compile method.

I could. Then I would have to move the .class files around.  It compiles a 
given source directory but I wanted to compile source files from the same 
directory in a different destination.   I could update the test either way.
Mandy

Re: RFR 8153123 : Streamline StackWalker code

[Brent Christian]

On 04/07/2016 04:33 PM, Brent Christian wrote:

I will send:
   http://cr.openjdk.java.net/~bchristi/8153123/webrev.02/
to hs-rt shortly.


...after adding the following :)

diff -r f628b87a6067 makefiles/symbols/symbols-unix
--- a/makefiles/symbols/symbols-unixFri Apr 08 13:14:23 2016 +0200
+++ b/makefiles/symbols/symbols-unixFri Apr 08 12:22:14 2016 -0700
@@ -58,7 +58,6 @@
 JVM_DumpAllStacks
 JVM_DumpThreads
 JVM_FillInStackTrace
-JVM_FillStackFrames
 JVM_FindClassFromCaller
 JVM_FindClassFromClass
 JVM_FindLibraryEntry
@@ -169,7 +168,6 @@
 JVM_ResumeThread
 JVM_SetArrayElement
 JVM_SetClassSigners
-JVM_SetMethodInfo
 JVM_SetNativeThreadName
 JVM_SetPrimitiveArrayElement
 JVM_SetThreadPriority
@@ -178,6 +176,7 @@
 JVM_StopThread
 JVM_SupportsCX8
 JVM_SuspendThread
+JVM_ToStackTraceElement
 JVM_TotalMemory
 JVM_UnloadLibrary
 JVM_Yield

Re: Review request 8153665: URLClassLoader.definePackage() not throwing expected IAE

[Mandy Chung]

Updated the test to use CompileUtils that is simple and clean:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153665/webrev.01/

Mandy

Expecting Integer.valueOf(String) to accept Literal format ...

[kedar mhaswade]

While discussing with colleagues, someone said:

However, my main gripe is about not supporting in String representation of
> an integer what is supported in its literal representation.
> Thus, Integer x = 1_000_000; is valid, whereas
> Integer.valueOf("1_000_000") is not. That sucks.


It seems to me that this is a reasonable expectation and has practical
benefits (e.g. accepting program arguments that are integers with _'s).

Supporting underscores in number literals (beginning JDK 7) was meant for
readability of the
​ ​
Java source
​ ​
code.
​Perhaps doing this correctly incurs unwarranted implementation complexity
in the JDK.​
As library writers however, how would you explain this mismatch?
​ ​
Was this side effect
​(arguably so) ​
considered at all?

Regards,
Kedar