Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Peter Levart]

Hi Mandy,

On 04/13/2016 01:55 AM, Mandy Chung wrote:

On Apr 12, 2016, at 6:16 AM, Peter Levart  wrote:

...

And there's also a question whether referenced types derived from method 
signatures should be checked for visibility from the specified class loader (in 
method validateProxyInterfaces). On one hand it is nice to ensure that later 
usage of the proxy class will not encounter problems of visibility of types in 
method signatures, but on the other, Java has been known to practice late 
binding and a normal class implementing an interface is loaded without problems 
although its method signatures reference types that are not visible from the 
class' class loader. Only when such method is called does the resolving happen 
and exception is thrown. So the question is whether such visibility checks are 
actually beneficial. For example, one could successfully use a proxy class by 
only invoking methods that reference visible types if this check was not 
performed.


I noticed this missing visibility check when updating proxies to work with 
modules.  I don’t know the history why it only checks of proxy interfaces when 
the API was defined.  In typical cases, when a proxy interface is visible to 
the specified class loader, the referenced types are likely visible. On the 
other hand, I’m cautious of the compatibility risk if the visibility check 
applies to referenced types as well. I don’t think this check has vast benefits 
while past proxy changes broke existing libraries that show up the 
incompatibility risk is somewhat hard to assess.


The problem is of course with the setup procedure of the dynamic module where 
you have to add exports from modules/packages of those referenced types and 
read edges to modules of those references types upfront. But this problem is 
resolvable. If a type is not visible from the proxy class' class loader, then 
we don't need an export from its module/package and we don't need a read edge 
to its module, do we?

True. If the type is not visible, NCFE will be thrown and this read edge and 
qualified exports would be redundant. Are you worrying the unused qualified 
exports causing security concerns?  The proxy classes in a dynamic module are 
encapsulate and I’m not too concerned of the redundant ones.



No, not about security. Mainly about binary compatibility. For example:

- library A v1 defines an interface I with some methods
- library B creates a dynamic proxy implementing I. It depends on 
library A and libraries defining types from method signatures of the 
interface

- program P uses B and depends on the transitive closure

now comes new version of library A v2 which adds a default method to 
interface I with signature that requires additional dependency which is 
tagged as "optional". Program P does not need to call this new method on 
the proxy created by B. Should we force P to bundle the new dependency 
although it is not used?


Regards, Peter

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Peter Levart]

On 04/13/2016 05:54 AM, Mandy Chung wrote:

Updated webrev to skip the static methods:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153895/webrev.01/

Mandy


Looks good.

Regards, Peter

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Sundararajan Athijegannathan]

+1

-Sundar

On 4/13/2016 9:24 AM, Mandy Chung wrote:
> Updated webrev to skip the static methods:
>http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153895/webrev.01/
>
> Mandy

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Mandy Chung]

Updated webrev to skip the static methods:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153895/webrev.01/

Mandy

JDK 9 RFR of JDK-4851642: Add fused mac to Java math library

[joe darcy]

Hello,

Please review the changes for

JDK-4851642: Add fused mac to Java math library
http://cr.openjdk.java.net/~darcy/4851642.0/

Fused mac (multiply-accumulate) is a ternary floating-point operation 
which accepts three inputs, a, b, c, and computes


a * b + c

with a single rounding error rather than the usual two rounding errors 
(a first for the multiply, a second one for the add). The fused mac 
operation was added in the 2008 update to the IEEE 754 floating-point 
standard and hardware support for the operation is becoming more and 
more common in different processor families.


When present as a hardware instruction, a fused mac can speed up loops 
such as those for polynomial evaluation. A fused mac can also be used to 
support a correctly rounding floating-point divide and support various 
higher-precision operations such as "doubled-double" arithmetic.


With the increasing availability of fused mac as a hardware primitive, 
the time has come to add fused mac to the Java math library. Fused mac 
is an ideal candidate to be intrinsified where hardware support is 
available. However, this initial implementation does not attempt to add 
any such intrinsics support in HotSpot; a follow-up RFE has been filed 
for that work (JDK-8154122). The current library implementation favors 
code simplicity over speed; a more performant implementation could be 
written by directly decomposing the floating-point inputs rather than 
turning to BigDecimal and may be written in the future. More extensive 
tests could be added in the future as well.


Thanks,

-Joe

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

[Mandy Chung]

> On Apr 12, 2016, at 1:34 AM, Rémi Forax  wrote:
> 
> Hi Mandy,
> I really don't like this patch.
> 
> Being forced to call toStackElement to get the line number is counter 
> intuitive.
> I would prefer the two methods to not return Optional but an int and a String 
> with the same convention as StackElement if the point of this patch is to 
> remove the dependency to Optional. 
> 

I was expecting the common usage of StackWalker API does not need file name and 
line number.  I think it'd be useful to include StackFrame::getBci (in the 
future it might include live information like locals etc) and keep the optional 
stuff and uncommon usage to StackTraceElement.

Mandy


> Rémi
> 
> 
> Le 11 avril 2016 23:22:39 CEST, Mandy Chung  a écrit :
>> Webrev at:
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.00/index.html
>> 
>> StackFrame::getFileName and StackFrame::getLineNumber are originally
>> proposed with the view of any stack walking code can migrate to the
>> StackWalker API without the use of StackTraceElement. 
>> 
>> File name and line number are useful for debugging and troubleshooting
>> purpose. It has additional overhead to map from a method and BCI to
>> look up the file name and line number. 
>> 
>> StackFrame::toStackTraceElement method returns StackTraceElement that
>> includes the file name and line number. There is no particular benefit
>> to duplicate getFileName and getLineNumber methods in StackFrame. It is
>> equivalently convenient to call
>> StackFrame.toStackTraceElement().getFileName() (or getLineNumber). 
>> 
>> This patch proposes to remove StackFrame::getFileName and
>> StackFrame::getLineNumber methods since such information can be
>> obtained from StackFrame.toStackTraceElement().
>> 
>> Mandy
> 

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Mandy Chung]

> On Apr 12, 2016, at 6:16 AM, Peter Levart  wrote:
> 
> Hi Mandy,
> 
> There's another redundant validation and read edge addition I think. When 
> deriving referenced types from methods of interfaces, static methods could be 
> skipped as they are not implemented or overridden by Proxy class:

I think static methods could be skipped.

> 
> http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.noStaticSignatures/webrev.01/
> 
> And there's also a question whether referenced types derived from method 
> signatures should be checked for visibility from the specified class loader 
> (in method validateProxyInterfaces). On one hand it is nice to ensure that 
> later usage of the proxy class will not encounter problems of visibility of 
> types in method signatures, but on the other, Java has been known to practice 
> late binding and a normal class implementing an interface is loaded without 
> problems although its method signatures reference types that are not visible 
> from the class' class loader. Only when such method is called does the 
> resolving happen and exception is thrown. So the question is whether such 
> visibility checks are actually beneficial. For example, one could 
> successfully use a proxy class by only invoking methods that reference 
> visible types if this check was not performed.
> 

I noticed this missing visibility check when updating proxies to work with 
modules.  I don’t know the history why it only checks of proxy interfaces when 
the API was defined.  In typical cases, when a proxy interface is visible to 
the specified class loader, the referenced types are likely visible. On the 
other hand, I’m cautious of the compatibility risk if the visibility check 
applies to referenced types as well. I don’t think this check has vast benefits 
while past proxy changes broke existing libraries that show up the 
incompatibility risk is somewhat hard to assess.

> The problem is of course with the setup procedure of the dynamic module where 
> you have to add exports from modules/packages of those referenced types and 
> read edges to modules of those references types upfront. But this problem is 
> resolvable. If a type is not visible from the proxy class' class loader, then 
> we don't need an export from its module/package and we don't need a read edge 
> to its module, do we?

True. If the type is not visible, NCFE will be thrown and this read edge and 
qualified exports would be redundant. Are you worrying the unused qualified 
exports causing security concerns?  The proxy classes in a dynamic module are 
encapsulate and I’m not too concerned of the redundant ones.

I’ll see what I would do for the static methods and send an updated webrev.

Mandy

> 
> What do you think?
> 
> Regards, Peter
> 
> On 04/12/2016 02:34 PM, Peter Levart wrote:
>> Hi Mandy,
>> 
>> This is OK, but the whole loop could be simplified. No need for Dequeue any 
>> more:
>> 
>>// set up proxy class access to proxy interfaces
>>Set> visited = new HashSet<>(interfaces);
>>for (Class c : visited) {
>>ensureAccess(target, c);
>>}
>> 
>> Regards, Peter
>> 
>> On 04/12/2016 01:45 AM, Mandy Chung wrote:
>>> Peter spots the redundant read edges are added to dynamic module when 
>>> creating a proxy class.   Proxy class does not access super interfaces of 
>>> proxy interfaces.   I have verified this simple patch with all core tests 
>>> and JCK api tests.
>>> 
>>> diff --git a/src/java.base/share/classes/java/lang/reflect/Proxy.java 
>>> b/src/java.base/share/classes/java/lang/reflect/Proxy.java
>>> --- a/src/java.base/share/classes/java/lang/reflect/Proxy.java
>>> +++ b/src/java.base/share/classes/java/lang/reflect/Proxy.java
>>> @@ -804,11 +804,6 @@
>>>  continue;
>>>  }
>>>  ensureAccess(target, c);
>>> -
>>> -// add all superinterfaces
>>> -for (Class intf : c.getInterfaces()) {
>>> -deque.add(intf);
>>> -}
>>>  }
>>>// set up proxy class access to types referenced in the 
>>> method signature
>> 
> 

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

[huizhe wang]

Thanks Lance!

On 4/12/2016 12:28 PM, Lance Andersen wrote:

Looks OK Joe.

Best
Lance
On Apr 8, 2016, at 4:49 PM, huizhe wang > wrote:


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




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: JDK-8153781 Issue in XMLScanner: EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET when skipping large DOCTYPE section with CRLF at wrong place

[huizhe wang]

On 4/12/2016 11:50 AM, Langer, Christoph wrote:

Hi Joe,

thanks as always for your suggestions and I'll try to work it in. It will 
probably take me a little while as I'm currently busy with another thing. I'll 
update my webrev eventually and add a testcase, too.


Ok.


But one question: I understand that the fix in skipDTD will be sufficient to 
fix the issue. Nevetheless, can you explain me why the change in scanData is 
not beneficial or could even cause issues? There are several places in scanData 
when further loads are done. But only at this point where there's exactly one 
character after CRLF at the end of the buffer the method just returns without 
further load. If it wouldn't leave here it seems to me as if it would continue 
correctly and load more data. That would probably also be better in the sense 
of performance I guess??


It's there to handle the situation where a surrogate pair got split in 
between buffers.


-Joe



Thanks
Christoph


-Original Message-
From: huizhe wang [mailto:huizhe.w...@oracle.com]
Sent: Dienstag, 12. April 2016 19:54
To: Langer, Christoph 
Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: JDK-8153781 Issue in XMLScanner:
EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET when skipping
large DOCTYPE section with CRLF at wrong place

Also, EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET was a
wrong msg
id. It would be good to change that to DoctypedeclNotClosed and add a
message to XMLMessages.properties right before DoctypedeclUnterminated,
sth. like the following:

DoctypedeclNotClosed = The document type declaration for root element
type \"{0}\" must be closed with '']''.

Thanks,
Joe

On 4/11/2016 5:49 PM, huizhe wang wrote:

On 4/7/2016 1:45 PM, Langer, Christoph wrote:

Hi,



I've run into a peculiar issue with Xerces.



The problem is happening when a DTD shall be skipped, the DTD is
larger than the buffer of the current entity and a CRLF sequence
occurs just one char before the buffer end.



The reason is that method skipDTD of class XMLDTDScannerImpl (about
line 389) calls XMLEntityScanner.scanData() to scan for the next
occurrence of ']'. The scanData method might return true which
indicates that the delimiter ']' was not found yet and more data is
to scan. Other users of scanData would handle this and call this
method in a loop until it returns false or some other condition
happens. So I've fixed that place like at the other callers of scanData.

This part of the change looks good.



Nevertheless, the scanData method would usually load more data when
it is at the end of the buffer. But in the special case when CRLF is
found at the end of buffer - 1, scanData would just return true. So I
also removed that check at line 1374 in XMLEntityScanner. Do you see
any problem with that? Is there any reason behind it which I'm
overseeing?

No need to remove this after the above change. The parser needs to
retain what's in the xml, e.g., not removing new lines.

Furthermore I took the chance for further little cleanups. I've added
the new copyright header to the files... is that the correct one?

Yes, that's the right license header. However,


I also aligned the calls to invokeListeners(position) in
XMLEntityScanner to always pass the actual position from which the
load is started. Do you think this is correct?

Yes.



Here is the bug:

https://bugs.openjdk.java.net/browse/JDK-8153781



Here is the webrev:

http://cr.openjdk.java.net/~clanger/webrevs/8153781.0/



Please give me some comments before I finalize my change including a
jtreg testcase.

It would be better if you had included the testcase so that the review
can be done together with the code change.

Thanks,
Joe




Thanks & Best regards

Christoph

Re: RFR: 8153293 - Stream API: Preserve SORTED and DISTINCT characteristics for boxed() and asLongStream() operations

[Stefan Zobel]

Hi Tagir,

thanks! Looks fine to me now - especially the new testSortDistinct() methods.
But note: I'm only a participant, not a reviewer of any sort.

Thanks,
Stefan


2016-04-12 18:37 GMT+02:00 Tagir F. Valeev :
> Hello!
>
> Thank you, Stefan and Paul for review. Here's updated webrev:
>
> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r2/
>
> Changes:
> - all new mapToObj are private and not final
> - IntStream.asDoubleStream preserves distinct now
> - Tests fixed according to Stefan's suggestions
> - Additional tests added which test how sorted and distinct actually
>   work, as Paul suggests. Values close to Integer.MAX_VALUE and Long.MAX_VALUE
>   are tested. TreeSet and TreeSet is used to produce
>   expected result.
>
> With best regards,
> Tagir Valeev.
>

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

[Lance Andersen]

Looks OK Joe.

Best
Lance
> On Apr 8, 2016, at 4:49 PM, huizhe wang  wrote:
> 
> 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

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: 8154067: Avoid early use of limited privilege escalation in InnerClassLambdaMetafactory

[Claes Redestad]

Mandy, Sean, Alan, Brian,

thanks for reviewing this!

/Claes

On 2016-04-12 20:29, Brian Goetz wrote:

No problem with this change.  +1.



On Apr 12, 2016, at 8:38 AM, Claes Redestad  wrote:

Hi,

the first usage of limited doPrivileged appears to have a small startup penalty 
(loads 8 permission-related classes and does some reflection), and is arguably 
excessive for this particular instance. Unrestricted doPrivileged allows for a 
small reduction of lambda init cost.

Webrev: http://cr.openjdk.java.net/~redestad/8154067/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8154067

Thanks!

/Claes

RE: RFR: JDK-8153781 Issue in XMLScanner: EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET when skipping large DOCTYPE section with CRLF at wrong place

[Langer, Christoph]

Hi Joe,

thanks as always for your suggestions and I'll try to work it in. It will 
probably take me a little while as I'm currently busy with another thing. I'll 
update my webrev eventually and add a testcase, too.

But one question: I understand that the fix in skipDTD will be sufficient to 
fix the issue. Nevetheless, can you explain me why the change in scanData is 
not beneficial or could even cause issues? There are several places in scanData 
when further loads are done. But only at this point where there's exactly one 
character after CRLF at the end of the buffer the method just returns without 
further load. If it wouldn't leave here it seems to me as if it would continue 
correctly and load more data. That would probably also be better in the sense 
of performance I guess??

Thanks
Christoph

> -Original Message-
> From: huizhe wang [mailto:huizhe.w...@oracle.com]
> Sent: Dienstag, 12. April 2016 19:54
> To: Langer, Christoph 
> Cc: core-libs-dev@openjdk.java.net
> Subject: Re: RFR: JDK-8153781 Issue in XMLScanner:
> EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET when skipping
> large DOCTYPE section with CRLF at wrong place
>
> Also, EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET was a
> wrong msg
> id. It would be good to change that to DoctypedeclNotClosed and add a
> message to XMLMessages.properties right before DoctypedeclUnterminated,
> sth. like the following:
>
> DoctypedeclNotClosed = The document type declaration for root element
> type \"{0}\" must be closed with '']''.
>
> Thanks,
> Joe
>
> On 4/11/2016 5:49 PM, huizhe wang wrote:
> >
> > On 4/7/2016 1:45 PM, Langer, Christoph wrote:
> >> Hi,
> >>
> >>
> >>
> >> I've run into a peculiar issue with Xerces.
> >>
> >>
> >>
> >> The problem is happening when a DTD shall be skipped, the DTD is
> >> larger than the buffer of the current entity and a CRLF sequence
> >> occurs just one char before the buffer end.
> >>
> >>
> >>
> >> The reason is that method skipDTD of class XMLDTDScannerImpl (about
> >> line 389) calls XMLEntityScanner.scanData() to scan for the next
> >> occurrence of ']'. The scanData method might return true which
> >> indicates that the delimiter ']' was not found yet and more data is
> >> to scan. Other users of scanData would handle this and call this
> >> method in a loop until it returns false or some other condition
> >> happens. So I've fixed that place like at the other callers of scanData.
> >
> > This part of the change looks good.
> >>
> >>
> >>
> >> Nevertheless, the scanData method would usually load more data when
> >> it is at the end of the buffer. But in the special case when CRLF is
> >> found at the end of buffer - 1, scanData would just return true. So I
> >> also removed that check at line 1374 in XMLEntityScanner. Do you see
> >> any problem with that? Is there any reason behind it which I'm
> >> overseeing?
> >
> > No need to remove this after the above change. The parser needs to
> > retain what's in the xml, e.g., not removing new lines.
> >>
> >> Furthermore I took the chance for further little cleanups. I've added
> >> the new copyright header to the files... is that the correct one?
> >
> > Yes, that's the right license header. However,
> >>
> >>
> >> I also aligned the calls to invokeListeners(position) in
> >> XMLEntityScanner to always pass the actual position from which the
> >> load is started. Do you think this is correct?
> >
> > Yes.
> >>
> >>
> >>
> >> Here is the bug:
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8153781
> >>
> >>
> >>
> >> Here is the webrev:
> >>
> >> http://cr.openjdk.java.net/~clanger/webrevs/8153781.0/
> >>
> >>
> >>
> >> Please give me some comments before I finalize my change including a
> >> jtreg testcase.
> >
> > It would be better if you had included the testcase so that the review
> > can be done together with the code change.
> >
> > Thanks,
> > Joe
> >
> >>
> >>
> >>
> >> Thanks & Best regards
> >>
> >> Christoph
> >>
> >>
> >>
> >

Re: RFR: 8154067: Avoid early use of limited privilege escalation in InnerClassLambdaMetafactory

[Brian Goetz]

No problem with this change.  +1.


> On Apr 12, 2016, at 8:38 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> the first usage of limited doPrivileged appears to have a small startup 
> penalty (loads 8 permission-related classes and does some reflection), and is 
> arguably excessive for this particular instance. Unrestricted doPrivileged 
> allows for a small reduction of lambda init cost.
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8154067/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8154067
> 
> Thanks!
> 
> /Claes

Re: RFR: 8154067: Avoid early use of limited privilege escalation in InnerClassLambdaMetafactory

[Sean Mullan]

On 04/12/2016 12:58 PM, Mandy Chung wrote:



On Apr 12, 2016, at 5:38 AM, Claes Redestad  wrote:

Hi,

the first usage of limited doPrivileged appears to have a small startup penalty 
(loads 8 permission-related classes and does some reflection), and is arguably 
excessive for this particular instance. Unrestricted doPrivileged allows for a 
small reduction of lambda init cost.

Webrev: http://cr.openjdk.java.net/~redestad/8154067/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8154067



+1

I agree that getting system properties isn’t a good usage example of limited 
doPrivileged since the permission is very clear and explicit.


I also agree. In this case, the privileged action (System.getProperty) 
is very concise, so the risk is very low that the additional permissions 
could be abused.


--Sean

Re: RFR: JDK-8153781 Issue in XMLScanner: EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET when skipping large DOCTYPE section with CRLF at wrong place

[huizhe wang]

Also, EXPECTED_SQUARE_BRACKET_TO_CLOSE_INTERNAL_SUBSET was a wrong msg 
id. It would be good to change that to DoctypedeclNotClosed and add a 
message to XMLMessages.properties right before DoctypedeclUnterminated, 
sth. like the following:


DoctypedeclNotClosed = The document type declaration for root element 
type \"{0}\" must be closed with '']''.


Thanks,
Joe

On 4/11/2016 5:49 PM, huizhe wang wrote:


On 4/7/2016 1:45 PM, Langer, Christoph wrote:

Hi,



I've run into a peculiar issue with Xerces.



The problem is happening when a DTD shall be skipped, the DTD is 
larger than the buffer of the current entity and a CRLF sequence 
occurs just one char before the buffer end.




The reason is that method skipDTD of class XMLDTDScannerImpl (about 
line 389) calls XMLEntityScanner.scanData() to scan for the next 
occurrence of ']'. The scanData method might return true which 
indicates that the delimiter ']' was not found yet and more data is 
to scan. Other users of scanData would handle this and call this 
method in a loop until it returns false or some other condition 
happens. So I've fixed that place like at the other callers of scanData.


This part of the change looks good.




Nevertheless, the scanData method would usually load more data when 
it is at the end of the buffer. But in the special case when CRLF is 
found at the end of buffer - 1, scanData would just return true. So I 
also removed that check at line 1374 in XMLEntityScanner. Do you see 
any problem with that? Is there any reason behind it which I'm 
overseeing?


No need to remove this after the above change. The parser needs to 
retain what's in the xml, e.g., not removing new lines.


Furthermore I took the chance for further little cleanups. I've added 
the new copyright header to the files... is that the correct one?


Yes, that's the right license header. However,



I also aligned the calls to invokeListeners(position) in 
XMLEntityScanner to always pass the actual position from which the 
load is started. Do you think this is correct?


Yes.




Here is the bug:

https://bugs.openjdk.java.net/browse/JDK-8153781



Here is the webrev:

http://cr.openjdk.java.net/~clanger/webrevs/8153781.0/



Please give me some comments before I finalize my change including a 
jtreg testcase.


It would be better if you had included the testcase so that the review 
can be done together with the code change.


Thanks,
Joe





Thanks & Best regards

Christoph

Re: RFR: 8154067: Avoid early use of limited privilege escalation in InnerClassLambdaMetafactory

[Mandy Chung]

> On Apr 12, 2016, at 5:38 AM, Claes Redestad  wrote:
> 
> Hi,
> 
> the first usage of limited doPrivileged appears to have a small startup 
> penalty (loads 8 permission-related classes and does some reflection), and is 
> arguably excessive for this particular instance. Unrestricted doPrivileged 
> allows for a small reduction of lambda init cost.
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8154067/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8154067
> 

+1

I agree that getting system properties isn’t a good usage example of limited 
doPrivileged since the permission is very clear and explicit.

Mandy

Re: RFR: 8153293 - Stream API: Preserve SORTED and DISTINCT characteristics for boxed() and asLongStream() operations

[Tagir F. Valeev]

Hello!

Thank you, Stefan and Paul for review. Here's updated webrev:

http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r2/

Changes:
- all new mapToObj are private and not final
- IntStream.asDoubleStream preserves distinct now
- Tests fixed according to Stefan's suggestions
- Additional tests added which test how sorted and distinct actually
  work, as Paul suggests. Values close to Integer.MAX_VALUE and Long.MAX_VALUE
  are tested. TreeSet and TreeSet is used to produce
  expected result.

With best regards,
Tagir Valeev.

SZ> Hi Tagir,

SZ> another minor issue. The testFlags() methods in IntPrimitiveOpsTests
SZ>  / LongPrimitiveOpsTests each have a duplicated assert:


SZ> IntPrimitiveOpsTests:

SZ> assertFalse(IntStreams.of(1, 10).boxed().spliterator()
SZ>   .hasCharacteristics(Spliterator.SORTED));


SZ> LongPrimitiveOpsTests:

SZ> assertFalse(LongStreams.of(1, 10).boxed().spliterator()
SZ>   .hasCharacteristics(Spliterator.SORTED));


SZ> The asserts for IntStreams.range(1, 10).asDoubleStream() would have
SZ> to be changed to account for DISTINCTness, of course.

SZ> Regards,
SZ> Stefan


SZ> 2016-04-01 18:25 GMT+02:00 Tagir F. Valeev :
>> Hello!
>>
>> Please review and sponsor the following patch:
>> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/r1/
>>
>> The patch preserves more characteristics on primitive stream
>> operations:
>> IntStream/LongStream/DoubleStream.boxed() preserves SORTED and DISTINCT
>> IntStream.asLongStream() preserves SORTED and DISTINCT
>> IntStream.asDoubleStream() and LongStream.asDoubleStream() preserves SORTED
>> (different longs can be converted into the same double, so DISTINCT is
>> not preserved here; not sure whether this is possible for ints)
>>
>> Fixing the boxed() case is especially important as distinct() for
>> primitive streams is implemented like boxed().distinct().unbox, so the
>> actual distinct() operation cannot take the advantage of DISTINCT flag
>> (skip the operation at all) or SORTED flag (switch to more efficient
>> implementation).
>>
>> Here's the small JMH benchmark which measures the performance boost of
>> quite common operation: sort the input numbers and leave only distinct
>> ones:
>> http://cr.openjdk.java.net/~tvaleev/webrev/8153293/jmh/
>>
>> new Random(1).ints(size).sorted().distinct().toArray()
>>
>> I've got the following results.
>>
>> 9ea+111:
>>
>> Benchmark(size)  Mode  Cnt  Score  Error  
>> Units
>> SortDistinctTest.sortedDistinct  10  avgt   30  0,612 ±0,004  
>> us/op
>> SortDistinctTest.sortedDistinct1000  avgt   30 92,848 ±1,039  
>> us/op
>> SortDistinctTest.sortedDistinct  10  avgt   30  32147,205 ± 3487,422  
>> us/op
>>
>> 9ea+111 patched:
>>
>> Benchmark(size)  Mode  Cnt ScoreError  Units
>> SortDistinctTest.sortedDistinct  10  avgt   30 0,435 ±  0,001  us/op
>> SortDistinctTest.sortedDistinct1000  avgt   3040,555 ±  0,772  us/op
>> SortDistinctTest.sortedDistinct  10  avgt   30  9031,651 ± 73,956  us/op
>>
>> With best regards,
>> Tagir Valeev.
>>

Request for comments -- resource reification vs. mrjar scheme for runtime versioning of multi-release jar files

[Steve Drach]

We’ve identified two possible ways to address issue JDK-8151542. 


One way is to append a #runtime fragment to the input URL in URLClassPath to 
convey to URLJarFile  that we want to have the JarFile opened with the 
parameter Release.RUNTIME, so any loaded classes are runtime versioned.  This 
is currently implemented and was integrated as part of issue JDK-8132734 
  For this case, a resource 
URL returned from ClassLoader.getResource(s) is reified, that is the returned 
URL does not have the #runtime fragment appended to it, instead the URL points 
directly to the desired entry.

The other way is to create a new URL scheme “mrjar” so that when URLJarFile 
sees the URL it knows to open the Jarfile with the Release.RUNTIME parameter.  
No fragment is required.  A returned resource URL will be of the form 
“mrjar:!/{entry}

We’ve put together the following list of pros/cons for each approach.  We’re 
soliciting feedback from the mailing list.  We currently have a working patch 
for the reification approach, but not one for the new scheme approach.

reification pros
—
. produces a standard/normal URL pointing directly to the jar entry
. the String equivalent is parsable so it shouldn’t affect legacy code

reification cons
—
. exposes the internals of a jar (i.e explicitly shows the META-INF/versions 
directory)
. the input URL is modified by attaching a #runtime fragment to it
. URL is not “portable” across jars as platform release version changes

mrjar scheme pros
—
. one URL that always points to runtime versioned entries
. the same URL (with entry appended) is returned by the getResource method
. portable across different platform releases
. jigsaw has also introduced a new URL scheme

mrjar scheme cons
—
. may break String based parsing of URLS
. non-standard URL scheme
. what does it mean with non MR files?
. we haven’t put together a prototype yet

As I said, we’re soliciting feedback from the list.  My personal opinion is 
that we should go with what we have, the reification approach, since it’s least 
likely to break existing code.

Re: RFR: 8153334: Replace BufferedInputStreams use of AtomicReferenceFieldUpdater with Unsafe

[Paul Sandoz]

> On 6 Apr 2016, at 16:45, Claes Redestad  wrote:
> 
> On 04/06/2016 04:09 PM, Paul Sandoz wrote:
>>> small streams become big rivers (i don't know the idiomatic sentence in 
>>> English, so it's a rough translation from a French idiom),
>>> "Death by a thousand cuts" is one of my favorites:).  A "flat profile" is 
>>> another description of a similar thing.
>>> 
>> I still remain unconvinced in this case that such changes warrant an 
>> increase in unsafe usage (temporary or otherwise).
>> 
> 
> I did not intend for this patch to spark any controversy - in my mind it was 
> just a rather straightforward and easy way to save a few Kbs (and some 
> theoretic startup time) on small program startup and I'm happy to withdraw it 
> based on the feedback.
> 

I don’t think there is any controversy. I just think we should place this one 
aside for the moment. We can revisit later on.

Paul.

> I do however think that reducing the dependency graph of things which are 
> loaded in this early has merits on its own, regardless of how much it 
> actually improves things. Using VHs here - or even in CHM - seems more 
> controversial to me than using Unsafe to take shortcuts in low-level class 
> libraries that need to boot fast and with as few dependencies as possible 
> (since that allows them to be used in more places).
> 
> Thanks!
> 
> /Claes

Re: RFR: 8153334: Replace BufferedInputStreams use of AtomicReferenceFieldUpdater with Unsafe

[Paul Sandoz]

> On 6 Apr 2016, at 16:52, Vitaly Davidovich  wrote:
> 
> So are there any bootstrap issues with VH that would preclude using it in 
> BufferedInputStream?

FWIW I did a quick perf experiment a while ago modifying A*FU to use VHs 
underneath and the VM booted just fine.


> Is that even known at this point?

Not precisely.


> You mentioned that VH was designed to minimize bootstrapping issues, but 
> where's the "minimized" line drawn?
> 

I want to draw the line where it’s possible to modify CHM to use VHs.

Paul.

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Peter Levart]

Mandy,

On 04/12/2016 03:16 PM, Peter Levart wrote:
The problem is of course with the setup procedure of the dynamic 
module where you have to add exports from modules/packages of those 
referenced types and read edges to modules of those references types 
upfront. But this problem is resolvable. If a type is not visible from 
the proxy class' class loader, then we don't need an export from its 
module/package and we don't need a read edge to its module, do we?


What do you think? 


If you think my claims make sense, then the following change could 
establish such policy:


http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.noStaticSignatures/webrev.02/

Regards, Peter

Re: RFR [9] 8150829: Enhanced drop-args, identity and default constant, varargs adjustment

[Paul Sandoz]

Hi,

Just minor comments, below.

Paul.


MethodHandle
—

 972 /**
 973   * Adapts this method handle to be {@linkplain #asVarargsCollector 
variable arity}
 974   * if the boolean flag is true, else {@linkplain #asFixedArity fixed 
arity}.
 975   * If the method handle is already of the proper arity mode, it is 
returned
 976   * unchanged.
 977   * This method is sometimes useful when adapting a method handle 
that
 978   * may be variable arity, to ensure that the resulting adapter is also
 979   * variable arity if and only if the original handle was.  For 
example,
 980   * this code changes the first argument of a handle

, {@code mh},

to {@code int} without
 981   * disturbing its variable arity property:
 982   * {@code 
mh.asType(mh.type().changeParameterType(0,int.class)).withVarargs(mh.isVarargsCollector())}

The above paragraph can be an @apiNote.

Also can you format the code block over two lines to emphasise the last call., 
otherwise i think it is harder to read.


 983   * @param makeVarargs true if the return method handle should have 
variable arity behavior
 984   * @return a method handle of the same type, with possibly adjusted 
variable arity behavior
 985   * @throws IllegalArgumentException if {@code makeVarargs} is true and
 986   * this method handle does not have a trailing array parameter
 987   * @since 9

Add

@see #asVarargsCollector
@see #asFixedArity

?

 988  */
 989  public MethodHandle withVarargs(boolean makeVarargs) {



MethodHandles
—

2387 /** Produces a constant method handle of the requested return type 
which

new line after ‘/**'


2388  * returns the default value for that type every time it is invoked.
2389  * The resulting constant method handle will have no side effects.
2390  * The returned method handle is equivalent to {@code 
empty(methodType(type))}.
2391  * It is also equivalent to {@code 
explicitCastArguments(constant(Object.class, null), methodType(type))},
2392  * since {@code explicitCastArguments} converts {@code null} to 
default values.

Is this method more efficient than the other two?


2393  * @param type the expected return type of the desired method handle
2394  * @return a constant method handle that takes no arguments and 
returns the default value of the given type (or void, if the type is void)

Can you format to be a little more consistent and not so long on the line 
length, as it becomes really tricky read.


2395  * @throws NullPointerException if the argument is null
2396  * @see MethodHandles#constant
2397  * @see MethodHandles#empty
2398  * @since 9
2399  */
2400 public static  MethodHandle zero(Class type) {
2401 Objects.requireNonNull(type);
2402 return type.isPrimitive() ?  zero(Wrapper.forPrimitiveType(type), 
type) : zero(Wrapper.OBJECT, type);
2403 }
2404


2409 /**
2410  * Produces a method handle of the requested type which ignores any 
arguments, does nothing,
2411  * and returns a suitable default depending on the return type.
2412  * That is, it returns a zero primitive value, a {@code null}, or 
{@code void}.
2413  * The returned method handle is equivalent to
2414  * {@code dropArguments(zero(type.returnType()), 0, 
type.parameterList())}.
2415  * 
2416  * Example:  Given a predicate and target, a useful "if-then" 
construct can be constructed as

s/Example:/@apiNote (same applies to the method dropArgumentsToMatch)

s/constructed/produced


2417  * {@code guardWithTest(pred, target, empty(target.type())}.
2418  * @param type the type of the desired method handle
2419  * @return a constant method handle of the given type, which returns a 
default value of the given return type
2420  * @throws NullPointerException if the argument is null
2421  * @see MethodHandles#zero
2422  * @see MethodHandles#constant
2423  * @since 9
2424  */
2425 public static  MethodHandle empty(MethodType type) {


2726 MethodHandle h0= constant(boolean.class, true);

Space before '='


ConstantIdentityMHTest
—

You should test the signatures and values for all primitives, ref and void.




> On 11 Apr 2016, at 07:47, shilpi.rast...@oracle.com wrote:
> 
> Gentle Reminder!
> 
>  Forwarded Message 
> Subject:  RFR [9] 8150829: Enhanced drop-args, identity and default 
> constant, varargs adjustment
> Date: Thu, 24 Mar 2016 11:18:56 +0530
> From: shilpi.rast...@oracle.com 
> Reply-To: core-libs-dev@openjdk.java.net
> To:   mlvm-...@openjdk.java.net
> 
> Hi All,
> 
> Please review the following-
> 
> 
> https://bugs.openjdk.java.net/browse/JDK-8150829
> http://cr.openjdk.java.net/~srastogi/8150829/webrev.04
> 
> 
> 
> Thanks,
> Shilpi
> 
> 
> 
> ___
> mlvm-dev mailing list
> mlvm-...@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Re: RFR: 8154067: Avoid early use of limited privilege escalation in InnerClassLambdaMetafactory

[Alan Bateman]

On 12/04/2016 13:38, Claes Redestad wrote:

Hi,

the first usage of limited doPrivileged appears to have a small 
startup penalty (loads 8 permission-related classes and does some 
reflection), and is arguably excessive for this particular instance. 
Unrestricted doPrivileged allows for a small reduction of lambda init 
cost.


Webrev: http://cr.openjdk.java.net/~redestad/8154067/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8154067
In general then we've been encouraging the use of limited doPrivileged 
where it make sense although for cases like this then it doesn't have 
much advantage. An alternative would be to just use System.getProperty 
then System.getSecurityManager() == null.


-Alan

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Peter Levart]

Hi Mandy,

There's another redundant validation and read edge addition I think. 
When deriving referenced types from methods of interfaces, static 
methods could be skipped as they are not implemented or overridden by 
Proxy class:


http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.noStaticSignatures/webrev.01/

And there's also a question whether referenced types derived from method 
signatures should be checked for visibility from the specified class 
loader (in method validateProxyInterfaces). On one hand it is nice to 
ensure that later usage of the proxy class will not encounter problems 
of visibility of types in method signatures, but on the other, Java has 
been known to practice late binding and a normal class implementing an 
interface is loaded without problems although its method signatures 
reference types that are not visible from the class' class loader. Only 
when such method is called does the resolving happen and exception is 
thrown. So the question is whether such visibility checks are actually 
beneficial. For example, one could successfully use a proxy class by 
only invoking methods that reference visible types if this check was not 
performed.


The problem is of course with the setup procedure of the dynamic module 
where you have to add exports from modules/packages of those referenced 
types and read edges to modules of those references types upfront. But 
this problem is resolvable. If a type is not visible from the proxy 
class' class loader, then we don't need an export from its 
module/package and we don't need a read edge to its module, do we?


What do you think?

Regards, Peter

On 04/12/2016 02:34 PM, Peter Levart wrote:

Hi Mandy,

This is OK, but the whole loop could be simplified. No need for 
Dequeue any more:


// set up proxy class access to proxy interfaces
Set> visited = new HashSet<>(interfaces);
for (Class c : visited) {
ensureAccess(target, c);
}

Regards, Peter

On 04/12/2016 01:45 AM, Mandy Chung wrote:
Peter spots the redundant read edges are added to dynamic module when 
creating a proxy class.   Proxy class does not access super 
interfaces of proxy interfaces.   I have verified this simple patch 
with all core tests and JCK api tests.


diff --git a/src/java.base/share/classes/java/lang/reflect/Proxy.java 
b/src/java.base/share/classes/java/lang/reflect/Proxy.java

--- a/src/java.base/share/classes/java/lang/reflect/Proxy.java
+++ b/src/java.base/share/classes/java/lang/reflect/Proxy.java
@@ -804,11 +804,6 @@
  continue;
  }
  ensureAccess(target, c);
-
-// add all superinterfaces
-for (Class intf : c.getInterfaces()) {
-deque.add(intf);
-}
  }
// set up proxy class access to types referenced in 
the method signature

RFR: 8154067: Avoid early use of limited privilege escalation in InnerClassLambdaMetafactory

[Claes Redestad]

Hi,

the first usage of limited doPrivileged appears to have a small startup 
penalty (loads 8 permission-related classes and does some reflection), 
and is arguably excessive for this particular instance. Unrestricted 
doPrivileged allows for a small reduction of lambda init cost.


Webrev: http://cr.openjdk.java.net/~redestad/8154067/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8154067

Thanks!

/Claes

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Peter Levart]

Hi Mandy,

This is OK, but the whole loop could be simplified. No need for Dequeue 
any more:


// set up proxy class access to proxy interfaces
Set> visited = new HashSet<>(interfaces);
for (Class c : visited) {
ensureAccess(target, c);
}

Regards, Peter

On 04/12/2016 01:45 AM, Mandy Chung wrote:

Peter spots the redundant read edges are added to dynamic module when creating 
a proxy class.   Proxy class does not access super interfaces of proxy 
interfaces.   I have verified this simple patch with all core tests and JCK api 
tests.

diff --git a/src/java.base/share/classes/java/lang/reflect/Proxy.java 
b/src/java.base/share/classes/java/lang/reflect/Proxy.java
--- a/src/java.base/share/classes/java/lang/reflect/Proxy.java
+++ b/src/java.base/share/classes/java/lang/reflect/Proxy.java
@@ -804,11 +804,6 @@
  continue;
  }
  ensureAccess(target, c);
-
-// add all superinterfaces
-for (Class intf : c.getInterfaces()) {
-deque.add(intf);
-}
  }
  
  // set up proxy class access to types referenced in the method signature

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

[Paul Sandoz]

> On 12 Apr 2016, at 10:18, Claes Redestad  wrote:
> 
> Hi Steve,
> 
> On 2016-04-11 23:21, Steve Drach wrote:
>> Hi,
>> 
>> I’ve updated the following patch, incorporating code by Claes Redestad to 
>> handle some corner cases while processing the attribute value.  Note that 
>> we’ve limited the location of the value part of the attribute to accommodate 
>> startup performance requirements.  It’s not without precedent as at least 
>> one other attribute is also limited in amount of whitespace surrounding the 
>> value.
>> 
>>> 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.01/index.html 
>> 
>> 
> 
> this looks good to me.

Same here, and i like the additional tests. I think we have beaten this into 
submission at this point :-)

Paul.

> Good catch to delete the new jars added by the test.
> 
> I'll sponsor this for you once tests look OK.
> 
> Thanks!
> 
> /Claes
> 
>> 
>> Thanks
>> Steve
> 

RE: RFR: 8151876: (tz) Support tzdata2016c

[Ramanand Patil]

Hi Roger,

I don't think this is covered in any alternate tests, because this Displayname 
format is newly introduced with tzdata2016b for the newly added TimeZones.

Anyway, I will double check on this to see if any test already covers this 
scenario otherwise I will update or add a new test case to cover this.


Regards,
Ramanand.

-Original Message-
From: Roger Riggs 
Sent: Monday, April 11, 2016 8:26 PM
To: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8151876: (tz) Support tzdata2016c

Hi Ramanand,

The change to the
test.java.time.format.TestZoneTextPrinterParser.test_ParseText
just eliminates the test.

Is there an alternate test that the formatter is returning the correct value 
for the GMT+/- cases?

Roger


On 4/11/2016 6:59 AM, Ramanand Patil wrote:
> Hi all,
>
> I would like someone from java.time to do a second review for this.
>
> Regards,
> Ramanand.
>
> -Original Message-
> From: Masayoshi Okutsu
> Sent: Tuesday, April 05, 2016 5:09 AM
> To: Ramanand Patil; i18n-...@openjdk.java.net
> Cc: core-libs-dev@openjdk.java.net
> Subject: Re: RFR: 8151876: (tz) Support tzdata2016c
>
> Looks good to me. But I'd like someone from java.time to review the changes 
> to see if it's OK for java.time.
>
> Masayoshi
>
> On 4/4/2016 6:50 PM, Ramanand Patil wrote:
>> Hi all,
>>
>> Please review the latest TZDATA (tzdata2016c) integration to JDK9.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8151876
>>
>> Webrev: http://cr.openjdk.java.net/~rpatil/8151876/webrev.00/
>>
>> All the TimeZone related tests are passed after integration.
>>
>>
>>
>> Please note that this patch includes both tzdata2016b and tzdata2016c 
>> integration. The tzdata2016b review was abandoned because tzdata2016c was 
>> already released.
>>
>> As suggested by Masayoshi, changes are made such that,  "GMT+hh:mm" is used 
>> for formatting of the newly added TimeZones in tzdata2016b.
>>
>> [This is done to accommodate the IANA's new trial system where the 
>> new zones use numeric time zone abbreviations like "+04" instead of 
>> invented abbreviations like "ASTT".]
>>
>>
>>
>> Regards,
>>
>> Ramanand.

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

[Rémi Forax]

Hi Mandy,
I really don't like this patch.

Being forced to call toStackElement to get the line number is counter intuitive.
I would prefer the two methods to not return Optional but an int and a String 
with the same convention as StackElement if the point of this patch is to 
remove the dependency to Optional. 

Rémi


Le 11 avril 2016 23:22:39 CEST, Mandy Chung  a écrit :
>Webrev at:
>http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.00/index.html
>
>StackFrame::getFileName and StackFrame::getLineNumber are originally
>proposed with the view of any stack walking code can migrate to the
>StackWalker API without the use of StackTraceElement. 
>
>File name and line number are useful for debugging and troubleshooting
>purpose. It has additional overhead to map from a method and BCI to
>look up the file name and line number. 
>
>StackFrame::toStackTraceElement method returns StackTraceElement that
>includes the file name and line number. There is no particular benefit
>to duplicate getFileName and getLineNumber methods in StackFrame. It is
>equivalently convenient to call
>StackFrame.toStackTraceElement().getFileName() (or getLineNumber). 
>
>This patch proposes to remove StackFrame::getFileName and
>StackFrame::getLineNumber methods since such information can be
>obtained from StackFrame.toStackTraceElement().
>
>Mandy

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

[Claes Redestad]

Hi Steve,

On 2016-04-11 23:21, Steve Drach wrote:

Hi,

I’ve updated the following patch, incorporating code by Claes Redestad 
to handle some corner cases while processing the attribute value. 
 Note that we’ve limited the location of the value part of the 
attribute to accommodate startup performance requirements.  It’s not 
without precedent as at least one other attribute is also limited in 
amount of whitespace surrounding the value.


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.01/index.html 





this looks good to me. Good catch to delete the new jars added by the test.

I'll sponsor this for you once tests look OK.

Thanks!

/Claes



Thanks
Steve

Re: Review request 8153895 (proxy) redundant read edges to superinterfaces of proxy interfaces

[Chris Hegarty]

Looks ok Mandy.

-Chris.

On 12 Apr 2016, at 00:45, Mandy Chung  wrote:

> Peter spots the redundant read edges are added to dynamic module when 
> creating a proxy class.   Proxy class does not access super interfaces of 
> proxy interfaces.   I have verified this simple patch with all core tests and 
> JCK api tests.
> 
> diff --git a/src/java.base/share/classes/java/lang/reflect/Proxy.java 
> b/src/java.base/share/classes/java/lang/reflect/Proxy.java
> --- a/src/java.base/share/classes/java/lang/reflect/Proxy.java
> +++ b/src/java.base/share/classes/java/lang/reflect/Proxy.java
> @@ -804,11 +804,6 @@
> continue;
> }
> ensureAccess(target, c);
> -
> -// add all superinterfaces
> -for (Class intf : c.getInterfaces()) {
> -deque.add(intf);
> -}
> }
> 
> // set up proxy class access to types referenced in the method 
> signature