Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[David Holmes]

Hi Mandy,

On 16/02/2018 1:20 PM, mandy chung wrote:

On 2/15/18 6:11 PM, David Holmes wrote:

Hi Mandy,

Good to see this go. A few minor comments.

First I've added comments on the CSR as some of the doc changes don't 
read correctly.




Thanks for catching it.  What do you think about this revision:

  *  All registered {@link #addShutdownHook shutdown hooks}, if any,
  * are started in some unspecified order and allowed to run concurrently
  * until they finish.  Once this is done the virtual machine
  * {@link #halt halts}.


This sounds good.


  *  If this method is invoked after the virtual machine has started
  * shutdown hooks then if shutdown hooks have already been run and
  * the status is nonzero then this method halts the virtual machine
  * with the given status code; otherwise, this method blocks indefinitely
  * (i.e. if shutdown hooks are being run or if shutdown hooks have already
  * been run and the status is zero).


This is still confusing to me (and I don't claim the original is not 
also confusing!). Do we actually care/distinguish between "hooks have 
been started" and "hooks have already been run" - which implies all 
user-defined hooks have finished? I think the only thing that is 
significant is whether hooks have been started, and then what the status 
code is. So I'd suggest simply:


  *  If this method is invoked after the virtual machine has started
  * shutdown hooks and the status is nonzero then this method halts the
  * virtual machine with the given status code. Otherwise, this method
  * blocks indefinitely.
?


I'll update the CSR once we agree on one version.


src/hotspot/share/runtime/thread.cpp

This comment doesn't read correctly:

! // won't be run.  Note that if a shutdown hook was registered
  // was called, the Shutdown class would have already been loaded
! // (Runtime.addShutdownHook will load it).

delete "was called, "



Deleted.


---

src/java.base/share/classes/java/lang/Shutdown.java

This was a bit confusing. :) I wasn't at all sure you needed the 
COMPLETED state (which is really a HOOKS_HAVE_BEEN_STARTED state). But 
I see it allows for a second exit(

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

On 2/15/18 7:29 PM, Martin Buchholz wrote:
This also changes the handling of (undeprecated) method 
runFinalization and that's obviously related but could be a separate 
changeset.




I agree I should separate the runFinalization change.  I leave it in 
this webrev for now.  I will send out the final webrevs before I push (I 
am feeling under the weather a bit now).



We'll need to merge with my own pending change to Finalizer.java.


Merged.  New version:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.02/

Mandy

Re: RFR: 8197812: (ref) Data race in Finalizer

[Martin Buchholz]

Thanks, Mandy, changeset pushed.  We are in agreement.

On Thu, Feb 15, 2018 at 7:31 PM, mandy chung  wrote:

>
>
> On 2/15/18 7:13 PM, Martin Buchholz wrote:
>
>
>
> On Thu, Feb 15, 2018 at 3:20 PM, mandy chung 
> wrote:
>
>>
>> I have posted RFR to remove runFinalizersOnExit.   I think your patch
>> would become cleaner as you may keep the remove method or inline in
>> runFinalizer method as your original patch has.
>>
>> Do you want to wait until JDK-8198249?  or proceed this patch without
>> waiting?
>>
>
> I'd like to commit this patch now, in part because we're actually
> backporting this as a local patch to make race detection easier.
>
> Regarding remove(), I think it's actually cleaner inline, but if it's kept
> a separate method, then make it return a boolean, as with Collection remove
> methods.
>
>
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Finalizer-data-race/
>
> I reviewed this version.  It looks okay.
>
> With the removal of runFinalizersOnExit, I will inline the deregistration
> part as part of runFinalizers after I rebase with your change.  Hope that's
> okay with you.
>
> Mandy
>

Re: RFR: 8197812: (ref) Data race in Finalizer

[mandy chung]

On 2/15/18 7:13 PM, Martin Buchholz wrote:



On Thu, Feb 15, 2018 at 3:20 PM, mandy chung > wrote:



I have posted RFR to remove runFinalizersOnExit.   I think your
patch would become cleaner as you may keep the remove method or
inline in runFinalizer method as your original patch has.

Do you want to wait until JDK-8198249?  or proceed this patch
without waiting?


I'd like to commit this patch now, in part because we're actually 
backporting this as a local patch to make race detection easier.


Regarding remove(), I think it's actually cleaner inline, but if it's 
kept a separate method, then make it return a boolean, as with 
Collection remove methods.




http://cr.openjdk.java.net/~martin/webrevs/jdk/Finalizer-data-race/

I reviewed this version.  It looks okay.

With the removal of runFinalizersOnExit, I will inline the 
deregistration part as part of runFinalizers after I rebase with your 
change.  Hope that's okay with you.


Mandy

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[Martin Buchholz]

This also changes the handling of (undeprecated) method runFinalization and
that's obviously related but could be a separate changeset.

We'll need to merge with my own pending change to Finalizer.java.

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

On 2/15/18 6:11 PM, David Holmes wrote:

Hi Mandy,

Good to see this go. A few minor comments.

First I've added comments on the CSR as some of the doc changes don't 
read correctly.




Thanks for catching it.  What do you think about this revision:

 *  All registered {@link #addShutdownHook shutdown hooks}, if any,
 * are started in some unspecified order and allowed to run concurrently
 * until they finish.  Once this is done the virtual machine
 * {@link #halt halts}.
 *
 *  If this method is invoked after the virtual machine has started
 * shutdown hooks then if shutdown hooks have already been run and
 * the status is nonzero then this method halts the virtual machine
 * with the given status code; otherwise, this method blocks indefinitely
 * (i.e. if shutdown hooks are being run or if shutdown hooks have already
 * been run and the status is zero).


I'll update the CSR once we agree on one version.


src/hotspot/share/runtime/thread.cpp

This comment doesn't read correctly:

! // won't be run.  Note that if a shutdown hook was registered
  // was called, the Shutdown class would have already been loaded
! // (Runtime.addShutdownHook will load it).

delete "was called, "



Deleted.


---

src/java.base/share/classes/java/lang/Shutdown.java

This was a bit confusing. :) I wasn't at all sure you needed the 
COMPLETED state (which is really a HOOKS_HAVE_BEEN_STARTED state). But 
I see it allows for a second exit(

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[Stuart Marks]

On 2/15/18 3:06 PM, mandy chung wrote:

Runtime.runFinalizersOnExit has been deprecated since 1.2 (1998)
and deprecated for removal in JDK 9.  We analyzed the maven central
artifacts few years ago that show very few uses of it.  I also
survey a recent version of most of the maven artifacts that references
runFinalizatsOnExit no longer references it.  I propose to remove
Runtime.runFinalizersOnExit and System.runFinalizersOnExit methods
in JDK 11.

CSR: https://bugs.openjdk.java.net/browse/JDK-8198250


I've added myself to the reviewers list of this CSR.

Note that you've listed the Compatibility Risk as Medium, but the notes are in 
support of a low risk level. Given this analysis, I'd be comfortable with 
Compatibility Risk being set to Low if you want to do that.



Webrev:
   http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.00/


Finalizer.java:


 111This method is used by both runFinalization.


Delete "both".

Thanks for updating the docs and comments in jdeprscan.

Overall looks good. Someone else should look at the native code, though, since 
I'm not too familiar with it.


s'marks

Re: RFR: 8197812: (ref) Data race in Finalizer

[Martin Buchholz]

On Thu, Feb 15, 2018 at 3:20 PM, mandy chung  wrote:

>
> I have posted RFR to remove runFinalizersOnExit.   I think your patch
> would become cleaner as you may keep the remove method or inline in
> runFinalizer method as your original patch has.
>
> Do you want to wait until JDK-8198249?  or proceed this patch without
> waiting?
>

I'd like to commit this patch now, in part because we're actually
backporting this as a local patch to make race detection easier.

Regarding remove(), I think it's actually cleaner inline, but if it's kept
a separate method, then make it return a boolean, as with Collection remove
methods.

Re: RFR: 8041626: [Event Request] Shutdown reason

[David Holmes]

Looks fine to me.

Thanks,
David
-

On 15/02/2018 11:35 PM, Robin Westberg wrote:

Hi again,

Here’s the (hopefully) final version:

Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ 

Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ 


Best regards,
Robin


On 14 Feb 2018, at 16:15, Robin Westberg  wrote:

Hi Roger,


On 13 Feb 2018, at 16:17, Roger Riggs  wrote:

Hi Robin,

It looks like the status argument to BeforeHalt is discarded in JVM_BeforeHalt
and is not inserted into the event.
That suggests it should be removed all the way back to Shutdown.beforeHalt.


You are right, my thinking was that the interface wouldn’t need to be changed 
if we decided to revisit the event in the future and add the status code. But 
that is perhaps unlikely, so can certainly remove the argument for now.

Best regards,
Robin



Roger



On 2/13/2018 9:59 AM, Robin Westberg wrote:

Hi Alan,


On 12 Feb 2018, at 09:02, Alan Bateman  wrote:



On 12/02/2018 07:07, David Holmes wrote:

Okay, I’ve removed the code related to the status field, certainly makes the 
change a bit less intrusive.

Updated webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/
Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/

This looks much cleaner/neater to me - thanks.


The updates to Runtime/Shutdown seems okay.

Thanks for reviewing!

Best regards,
Robin


-Alan

Re: JEP [DRAFT]: Container aware Java

[David Holmes]

On 16/02/2018 4:30 AM, kedar mhaswade wrote:

This appears useful.

Will Runtime.getRuntime().availableProcessors() return the processors
available to the container after this integration, or will a new API be
provided? I have seen thread pools being misconfigured (e.g. it returns the
number of processors available on the host which is far more than that of
processors allotted by cgroups to the container) because of the confusion
(or because of just using the non-container-aware code).


Depending on what you mean by "allotted" the VM has reported the actual 
set of CPU's available to the VM process (as can be configured by 
cpu_sets) since JDK 9. In JDK 10 quotas/shares are also used to 
approximate a notion of "available processors".


David


Regards,
Kedar

On Thu, Feb 15, 2018 at 10:04 AM, Bob Vandette 
wrote:




On Feb 15, 2018, at 12:52 PM, Volker Simonis 

wrote:


Sounds cool!

Is this JEP only about providing the corresponding API or also about
using it internally (within HotSpot and class library) to better adopt
to environment the JVM is running in?


Thanks Volker.

I integrated JDK-8146115 into JDK 10 which allows Hotspot to adapt to the
container it’s running in.  The configuration that is examined includes
memory limits,
cpu count including cpu shares and quotas.

This JEP’s main focus is to provide an internal API that can be used by
JDK tools
such as JFR and JMX to export container statistics and configuration
data.  This JEP
does not include the JFR and JMX implementations.  They will hopefully
follow shortly after.

Bob.



Either way, looking forward to see (and test) the first implementation

bits!


Regards,
Volker


On Thu, Feb 15, 2018 at 6:07 PM, Bob Vandette 

wrote:

I’d like to re-propose the following JEP that will enhance the Java

runtime to be more container aware.

This will add an Internal Java API that will provide container specific

statistics.  Some of the initial goals

of the previous JEP proposal has been integrated into JDK 10 under an

RFE (JDK-8146115).

This JEP is now focused on providing a Java API that exports Container

runtime configuration and metrics.


Since the scope of this JEP have changed, I’m re-submitting it for

comment and endorsement.



JEP Issue:

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

Here’s a Text dump of the JEP contents for your convenience:

Summary
---

Container aware Java runtime

Goals
-

Provide an internal API that can be used to extract container specific

configuration and runtime statistics.  This JEP will only support Docker on
Linux-x64 although the design should be flexible enough to allow support
for other platforms and container technologies.  The initial focus will be
on Linux cgroups technology so that we will be able to easily support other
container technologies running on Linux in addition to Docker.


Non-Goals
-

It is not a goal of this JEP to support any platform other than Docker

container technology running on Linux x64.


Success Metrics
---

Success will be measured by the improvement in information that will be

available to tools which visualize resource usage of containers that are
running Java processes.


Motivation
--

Container technology is becoming more and more prevalent in Cloud based

applications.  The Cloud Serverless application programming model motivates
developers to split large monolithic applications into 100s of smaller
pieces each running in thier own container.  This move increases the
importance of the observability of each running container process.  Adding
the proposed set of APIs will allow more details related to each container
process to be made available to external tools thereby improving the
observability.


Description
---

This enhancement will be made up of the following work items:

A. Detecting if Java is running in a container.

The Java runtime, as well as any tests that we might write for this

feature, will need to be able to detect that the current Java process is
running in a container.  A new API will be made available for this purpose.


B. Exposing container resource limits, configuration and runtime

statistics.


There are several configuration options and limits that can be imposed

upon a running container.  Not all of these

are important to a running Java process.  We clearly want to be able to

detect how many CPUs have been allocated to our process along with the
maximum amount of memory that the process has been allocated but there are
other options that we might want to base runtime decisions on.


In addition, since Container typically impose limits on system

resources, they also provide the ability to easily access the amount of
consumption of these resources.  The goal is to provide this information in
addition to the configuration data.


I propose adding a new jdk.internal.Platform class that will allow

access to 

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[David Holmes]

Hi Mandy,

Good to see this go. A few minor comments.

First I've added comments on the CSR as some of the doc changes don't 
read correctly.


src/hotspot/share/runtime/thread.cpp

This comment doesn't read correctly:

! // won't be run.  Note that if a shutdown hook was registered
  // was called, the Shutdown class would have already been loaded
! // (Runtime.addShutdownHook will load it).

delete "was called, "

---

src/java.base/share/classes/java/lang/Shutdown.java

This was a bit confusing. :) I wasn't at all sure you needed the 
COMPLETED state (which is really a HOOKS_HAVE_BEEN_STARTED state). But I 
see it allows for a second exit(

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

On 2/15/18 4:18 PM, Stuart Marks wrote:



On 2/15/18 3:06 PM, mandy chung wrote:

Runtime.runFinalizersOnExit has been deprecated since 1.2 (1998)
and deprecated for removal in JDK 9.  We analyzed the maven central
artifacts few years ago that show very few uses of it.  I also
survey a recent version of most of the maven artifacts that references
runFinalizatsOnExit no longer references it.  I propose to remove
Runtime.runFinalizersOnExit and System.runFinalizersOnExit methods
in JDK 11.

CSR: https://bugs.openjdk.java.net/browse/JDK-8198250


I've added myself to the reviewers list of this CSR.

Note that you've listed the Compatibility Risk as Medium, but the 
notes are in support of a low risk level. Given this analysis, I'd be 
comfortable with Compatibility Risk being set to Low if you want to do 
that.




I should make it clear.  I keep it medium because the data doesn't 
represent all libraries.



Webrev:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.00/


Finalizer.java:


 111    This method is used by both runFinalization.


Delete "both".



I will fix that.


Thanks for updating the docs and comments in jdeprscan.

Overall looks good. Someone else should look at the native code, 
though, since I'm not too familiar with it.



Thanks
Mandy

Re: RFR 8196298 Add null Reader and Writer

[Patrick Reinhart]

Hi everyone responded so far.

How should we proceed now?

-Patrick

Am 07.02.2018 um 18:38 schrieb Patrick Reinhart:
>> Am 07.02.2018 um 14:03 schrieb Alan Bateman :
>>
>> On 03/02/2018 17:05, Patrick Reinhart wrote:
>>> :
>>> I reworked the tests and Writer implementation accordingly
>>>
>>> http://cr.openjdk.java.net/~reinhapa/reviews/8196298/webrev.01
>>>
>> Just catching up on this.
>>
>> nullReader’s javadoc suggests that mark(int) does not nothing but this seems 
>> to conflict with the Reader's javadoc where it is specified to throw IOE 
>> when mark is not supported.
> So I will change the API description accordingly there…
>
>> I'm a bit uneasy with len==0 check in the read method. I realize this is 
>> trying to mirror InputStream.read and maybe some Reader implementations but 
>> it's not specified behavior. I think we have to re-examine the Reader spec 
>> to clarify this to avoid creating more inconsistencies.
> That’s exactly what I did, so for me it’s ok to look into the Reader spec 
> also as I already modify that File. I personally thought that a null like 
> reader will always be at the end of the stream and therefore return -1 and do 
> no checking of how much space may be left on the consuming end. That seem to 
> me more the natural way of thinking.
>
>> Similarly in nullWriter() where it looks like the the len==0 checks is in 
>> unspecified territory. I think this will need clarifications to the Writer 
>> spec. One obvious inconsistency is to close the writer and call write("") 
>> and write("",  0, 0). The first will fail as the writer is closed, the 
>> second does not fail.
> Here I think the same holds true as I wrote for the Reader…
>
>> Another one is read(CharBuffer). Suppose CharBuffer cb has 0 bytes 
>> remaining; if I invoke nullReader().read(cb) then it looks like it will 
>> return 0 whereas the read(CharBuffer) method is specified to return -1.
>>
> I see your point there, the implementation I did there was taken from the 
> StringReader, where it will be the same len==0 check as you mentioned before… 
> So it looks like we really need to look into those implementations…
>
> -Patrick
>
>

Re: RFR: JDK-8021560,(str) String constructors that take ByteBuffer

[Xueming Shen]

On 2/15/18, 1:55 PM, Stuart Marks wrote:



On 2/13/18, 12:41 AM, Alan Bateman wrote:
These four methods encode as many characters as possible into the 
destination byte[] or buffer but don't give any indication that the 
destination didn't have enough space to encode the entire string. I 
thus worry they could be a hazard and result in buggy code. If there 
is insufficient space then the user of the API doesn't know how many 
characters were encoded so it's not easy to substring and call 
getBytes again to encode the remaining characters. There is also the 
issue of how to size the destination. What would you think about 
having them fail when there is insufficient space? If they do fail 
then there is a side effect that they will have written to the 
destination so that would need to be documented too.


I share Alan's concern here.

If the intent is to reuse a byte[] or a ByteBuffer, then there needs 
to be an effective way to handle the case where the provided 
array/buffer doesn't have enough room to receive the decoded string. A 
variety of ways of dealing with this have been mentioned, such as 
throwing an exception; returning negative value to indicate failure, 


To add the ref for the discussion. Here is one of the alternatives being 
discussed,
to return the "-length", negative value of the space needed to encode 
the whole
String, to hint the caller to pass in an appropriately sized buffer 
(only the String
APIs are updated). This appears to be a better than the proposed 
"partial fill"/encode
as many as possible approach. But it will still force the caller to take 
care of the
return value and manage the possible buffer size issue himself, which 
triggers the
question that whether or not the CharsetDecoder is a better place for 
such kind of

use scenario.

http://cr.openjdk.java.net/~sherman/8021560/webrev.v2

-Sherman

Re: RFR: 8197812: (ref) Data race in Finalizer

[mandy chung]

Hi Martin,

On 2/14/18 9:47 PM, Martin Buchholz wrote:
Embarrassed by my failure to take runFinalizersOnExit into account, I 
tried to redo the patch to be actually correct.
http://cr.openjdk.java.net/~martin/webrevs/jdk/Finalizer-data-race/ 

even though we may succeed in making runFinalizersOnExit go away, and 
I'm not planning to submit any time soon.
The runFinalizersOnExit case needs different mechanics for removing 
elements from "unfinalized".
The simple invariant is that you need to run the finalizer whenever an 
element is unlinked from unfinalized.




I have posted RFR to remove runFinalizersOnExit.   I think your patch 
would become cleaner as you may keep the remove method or inline in 
runFinalizer method as your original patch has.


Do you want to wait until JDK-8198249?  or proceed this patch without 
waiting?


Mandy
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051518.html

[11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

Runtime.runFinalizersOnExit has been deprecated since 1.2 (1998)
and deprecated for removal in JDK 9.  We analyzed the maven central
artifacts few years ago that show very few uses of it.  I also
survey a recent version of most of the maven artifacts that references
runFinalizatsOnExit no longer references it.  I propose to remove
Runtime.runFinalizersOnExit and System.runFinalizersOnExit methods
in JDK 11.

 
CSR: https://bugs.openjdk.java.net/browse/JDK-8198250
 
Webrev:

  http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.00/

Thanks
Mandy
[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-January/031041.html

Re: RFR: 8197594 - String and character repeat

[Joseph D. Darcy]

On 2/15/2018 12:38 PM, Roger Riggs wrote:

Hi Jim,

Its cleaner to do the API (CSR) review before and without the 
implementation.

It helps keep the issues separate.

Don't make statements about count == Integer.MAX_VALUE / 2.
There is no point, unless it should throw IAE.



My general recommendation if the code review and CSR review are to be 
serialized is to estimate which one is more likely to generate feedback 
that might modify the proposal and run though that process first. Since 
there has already been feedback leading to updates of this proposal, 
code review first in this case seems like a reasonable choice :-)


Developers at their discretion can run CSR and code review in parallel, 
but feedback may come from either process. Proposals still under 
development can also go through the two-phase CSR process to get some 
initial feedback before an intended-to-be-final version is produced.


HTH,

-Joe

Re: RFR: JDK-8021560,(str) String constructors that take ByteBuffer

[Stuart Marks]

public String(ByteBuffer bytes, Charset cs);
public String(ByteBuffer bytes, String csname);


I think these constructors make good sense. They avoid an extra copy to an 
intermediate byte[].


One issue (also mentioned by Stephen Colebourne) is whether we need the csname 
overload. Arguably it's not needed if we have the Charset overload. And the 
csname overload throws UnsupportedEncodingException, which is checked. But the 
csname overload is apparently faster, since the decoder can be cached, and it's 
unclear when this can be remedied for the Charset case


I could go either way on this one.

**

I'd also suggest adding a CharBuffer constructor:

public String(CharBuffer cbuf)

This would be semantically equivalent to

public String(char[] value, int offset, int count)

except using the chars from the CharBuffer between the buffer's position and its 
limit.


**

Regarding the getBytes() overloads:


public int getBytes(byte[] dst, int offset, Charset cs);
public int getBytes(byte[] dst, int offset, String csname);
public int getBytes(ByteBuffer bytes, Charset cs);
public int getBytes(ByteBuffer bytes, Charset csn);



On 2/13/18, 12:41 AM, Alan Bateman wrote:
These four methods encode as many characters as possible into the destination 
byte[] or buffer but don't give any indication that the destination didn't 
have enough space to encode the entire string. I thus worry they could be a 
hazard and result in buggy code. If there is insufficient space then the user 
of the API doesn't know how many characters were encoded so it's not easy to 
substring and call getBytes again to encode the remaining characters. There is 
also the issue of how to size the destination. What would you think about 
having them fail when there is insufficient space? If they do fail then there 
is a side effect that they will have written to the destination so that would 
need to be documented too.


I share Alan's concern here.

If the intent is to reuse a byte[] or a ByteBuffer, then there needs to be an 
effective way to handle the case where the provided array/buffer doesn't have 
enough room to receive the decoded string. A variety of ways of dealing with 
this have been mentioned, such as throwing an exception; returning negative 
value to indicate failure, possibly also encoding the number of bytes written; 
or even allocating a fresh array or buffer of the proper size and returning that 
instead. The caller would have to check the return value and take care to handle 
all the cases properly. This is likely to be fairly error-prone.


This also raises the question in my mind of what these getBytes() methods are 
intended for.


On the one hand, they might be useful for the caller to manage its own memory 
allocation and reuse of arrays/buffers. If so, then it's necessary for 
intermediate results from partial processing to be handled properly. If the 
destination fills up, there needs to be a way to report how much of the input 
was consumed, so that a subsequent operation can pick up where the previous one 
left off. (This was one of David Lloyd's points.) If there's sufficient room in 
the destination, there needs to be a way to report this and how much space 
remains in the destination. One could contemplate adding all this information to 
the API. This eventually leads to


CharsetEncoder.encode(CharBuffer in, ByteBuffer out, boolean endOfInput)

which has all the necessary partial progress state in the buffers.

On the other hand, maybe the intent of these APIs is for convenience. I'd 
observe that String already has this method:


public byte[] getBytes(Charset)

which returns the decoded bytes in a newly allocated array of the proper size. 
This is pretty convenient. It doesn't let the caller reuse a destination array 
or buffer... but that's what brings in all the partial result edge cases.


Bottom line is that I'm not entirely sure of the use of these new getBytes() 
overloads. Maybe I've missed a use case where these work; if so, maybe somebody 
can describe it.


s'marks

Re: RFR: 8197594 - String and character repeat

[Brian Goetz]

This matches my intuition too.  

Sent from my MacBook Wheel

> On Feb 15, 2018, at 12:52 PM, Louis Wasserman  wrote:
> 
> I don't think there's a case for demand to merit having a
> repeat(CharSequence, int) at all.
> 
> I did an analysis of usages of Guava's Strings.repeat on Google's
> codebase.  Users might be rolling their own implementations, too, but this
> should be a very good proxy for demand.
> 
> StringRepeat_SingleConstantChar = 4.475 K // strings with .length() ==  1
> StringRepeat_SingleConstantCodePoint = 28 // strings with
> .codePointCount(...) == 1
> StringRepeat_MultiCodePointConstant = 1.156 K // constant strings neither
> of the above
> StringRepeat_CharSequenceToString = 2 //
> Strings.repeat(CharSequence.toString(), n)
> StringRepeat_NoneOfTheAbove = 248
> 
> Notably, it seems like basically nobody needs to repeat a CharSequence --
> definitely not enough demand to merit the awkwardness of e.g.
> Rope.repeat(n) inheriting a repeat returning a String.
> 
> Based on this data, I'd recommend providing one and only one method of this
> type: String.repeat(int).  There's no real advantage to a static
> repeat(char, int) method when the overwhelming majority of these are
> constants: e.g. compare SomeUtilClass.repeat('*', n) versus "*".repeat(n).
> Character.toString(c).repeat(n) isn't a bad workaround if you don't have a
> constant char.  There also isn't much demand for dealing with the code
> point case specially; the String.repeat(int) method seems like it'd handle
> that just fine.
> 
>> On Thu, Feb 15, 2018 at 11:44 AM Jim Laskey  wrote:
>> 
>> 
>> 
>>> On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov 
>> wrote:
>>> 
>>> Hello!
>>> 
>>> The link with the webrev returned 404, but I could find it at this
>> location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
>>> 
>>> A few minor comments:
>>> 
>>> 1)
>>> 
>>> This check:
>>> 
>>> 2992 final long limit = (long)count * 2L;
>>> 2993 if ((long)Integer.MAX_VALUE < limit) {
>>> 
>>> can be possibly simplified as
>>> if (count > Integer.MAX_VALUE - count) {
>> 
>> Good.
>> 
>>> 
>>> 2)
>>> Should String repeat(final int codepoint, final int count) be optimized
>> for codepoints that can be represented with a single char?
>>> 
>>> E.g. like this:
>>> 
>>> public static String repeat(final int codepoint, final int count) {
>>>   return Character.isBmpCodePoint(codepoint))
>>>   ? repeat((char) codepoint, count)
>>>   : (new String(Character.toChars(codepoint))).repeat(count);
>>> }
>> 
>> Yes, avoid array allocation.
>> 
>>> 
>>> 3)
>>> Using long arithmetic can possibly be avoided in the common path of
>> repeat(final int count):
>>> 
>>> E.g. like this:
>>> 
>>>if (count < 0) {
>>>throw new IllegalArgumentException("count is negative, " +
>> count);
>>>} else if (count == 1) {
>>>return this;
>>>} else if (count == 0) {
>>>return "";
>>> }
>>>final int len = value.length;
>>>if (Integer.MAX_VALUE / count < len) {
>>>throw new IllegalArgumentException(
>>>"Resulting string exceeds maximum string length: " +
>> ((long)len * (long)count));
>>>}
>>>final int limit = count * len;
>> 
>> Good.
>> 
>> Thank you.
>> 
>>> 
>>> With kind regards,
>>> Ivan
>>> 
 On 2/15/18 9:20 AM, Jim Laskey wrote:
 This is a pre-CSR code review [1] for String repeat methods
>> (Enhancement).
 
 The proposal is to introduce four new methods;
 
 1. public String repeat(final int count)
 2. public static String repeat(final char ch, final int count)
 3. public static String repeat(final int codepoint, final int count)
 4. public static String repeat(final CharSequence seq, final int count)
 
 For the sake of transparency, only 1 is necessary, 2-4 are convenience
>> methods.
 In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’,
>> 10).
 3 and 4 convert to String before calling 1.
 
 Performance runs with jmh (results as comment in [2]) show that these
 methods are significantly faster that StringBuilder equivalents.
 - fewer memory allocations
 - fewer char to byte array conversions
 - faster pyramid replication vs O(N) copying
 
 I left StringBuilder out of scope. It falls under the category of
 Appendables#append with repeat. A much bigger project.
 
 All comments welcome. Especially around the need for convenience
 methods, the JavaDoc content and expanding the tests.
 
 — Jim
 
 [1] webrev:
>> http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
 [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
 
 
>>> 
>>> --
>>> With kind regards,
>>> Ivan Gerasimov
>>> 
>> 
>> 

Re: RFR: 8197594 - String and character repeat

[James Laskey]

Good to have the data. Thank you Louis. 

Sent from my iPhone

> On Feb 15, 2018, at 4:52 PM, Louis Wasserman  wrote:
> 
> I don't think there's a case for demand to merit having a 
> repeat(CharSequence, int) at all.  
> 
> I did an analysis of usages of Guava's Strings.repeat on Google's codebase.  
> Users might be rolling their own implementations, too, but this should be a 
> very good proxy for demand.
> 
> StringRepeat_SingleConstantChar   = 4.475 K // strings with .length() ==  
> 1
> StringRepeat_SingleConstantCodePoint  = 28 // strings with 
> .codePointCount(...) == 1
> StringRepeat_MultiCodePointConstant   = 1.156 K // constant strings neither 
> of the above
> StringRepeat_CharSequenceToString = 2 // 
> Strings.repeat(CharSequence.toString(), n)
> StringRepeat_NoneOfTheAbove   = 248
> 
> Notably, it seems like basically nobody needs to repeat a CharSequence -- 
> definitely not enough demand to merit the awkwardness of e.g. Rope.repeat(n) 
> inheriting a repeat returning a String.
> 
> Based on this data, I'd recommend providing one and only one method of this 
> type: String.repeat(int).  There's no real advantage to a static repeat(char, 
> int) method when the overwhelming majority of these are constants: e.g. 
> compare SomeUtilClass.repeat('*', n) versus "*".repeat(n).  
> Character.toString(c).repeat(n) isn't a bad workaround if you don't have a 
> constant char.  There also isn't much demand for dealing with the code point 
> case specially; the String.repeat(int) method seems like it'd handle that 
> just fine.
> 
>> On Thu, Feb 15, 2018 at 11:44 AM Jim Laskey  wrote:
>> 
>> 
>> > On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov  
>> > wrote:
>> >
>> > Hello!
>> >
>> > The link with the webrev returned 404, but I could find it at this 
>> > location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
>> >
>> > A few minor comments:
>> >
>> > 1)
>> >
>> > This check:
>> >
>> > 2992 final long limit = (long)count * 2L;
>> > 2993 if ((long)Integer.MAX_VALUE < limit) {
>> >
>> > can be possibly simplified as
>> > if (count > Integer.MAX_VALUE - count) {
>> 
>> Good.
>> 
>> >
>> > 2)
>> > Should String repeat(final int codepoint, final int count) be optimized 
>> > for codepoints that can be represented with a single char?
>> >
>> > E.g. like this:
>> >
>> > public static String repeat(final int codepoint, final int count) {
>> >return Character.isBmpCodePoint(codepoint))
>> >? repeat((char) codepoint, count)
>> >: (new String(Character.toChars(codepoint))).repeat(count);
>> > }
>> 
>> Yes, avoid array allocation.
>> 
>> >
>> > 3)
>> > Using long arithmetic can possibly be avoided in the common path of 
>> > repeat(final int count):
>> >
>> > E.g. like this:
>> >
>> > if (count < 0) {
>> > throw new IllegalArgumentException("count is negative, " + 
>> > count);
>> > } else if (count == 1) {
>> > return this;
>> > } else if (count == 0) {
>> > return "";
>> > }
>> > final int len = value.length;
>> > if (Integer.MAX_VALUE / count < len) {
>> > throw new IllegalArgumentException(
>> > "Resulting string exceeds maximum string length: " + 
>> > ((long)len * (long)count));
>> > }
>> > final int limit = count * len;
>> 
>> Good.
>> 
>> Thank you.
>> 
>> >
>> > With kind regards,
>> > Ivan
>> >
>> > On 2/15/18 9:20 AM, Jim Laskey wrote:
>> >> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>> >>
>> >> The proposal is to introduce four new methods;
>> >>
>> >> 1. public String repeat(final int count)
>> >> 2. public static String repeat(final char ch, final int count)
>> >> 3. public static String repeat(final int codepoint, final int count)
>> >> 4. public static String repeat(final CharSequence seq, final int count)
>> >>
>> >> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>> >> methods.
>> >> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 
>> >> 10).
>> >> 3 and 4 convert to String before calling 1.
>> >>
>> >> Performance runs with jmh (results as comment in [2]) show that these
>> >> methods are significantly faster that StringBuilder equivalents.
>> >>  - fewer memory allocations
>> >>  - fewer char to byte array conversions
>> >>  - faster pyramid replication vs O(N) copying
>> >>
>> >> I left StringBuilder out of scope. It falls under the category of
>> >> Appendables#append with repeat. A much bigger project.
>> >>
>> >> All comments welcome. Especially around the need for convenience
>> >> methods, the JavaDoc content and expanding the tests.
>> >>
>> >> — Jim
>> >>
>> >> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>> >> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>> >>
>> >>
>> >
>> > --
>> > With kind regards,
>> > Ivan 

Re: RFR: 8197594 - String and character repeat

[Louis Wasserman]

I don't think there's a case for demand to merit having a
repeat(CharSequence, int) at all.

I did an analysis of usages of Guava's Strings.repeat on Google's
codebase.  Users might be rolling their own implementations, too, but this
should be a very good proxy for demand.

StringRepeat_SingleConstantChar = 4.475 K // strings with .length() ==  1
StringRepeat_SingleConstantCodePoint = 28 // strings with
.codePointCount(...) == 1
StringRepeat_MultiCodePointConstant = 1.156 K // constant strings neither
of the above
StringRepeat_CharSequenceToString = 2 //
Strings.repeat(CharSequence.toString(), n)
StringRepeat_NoneOfTheAbove = 248

Notably, it seems like basically nobody needs to repeat a CharSequence --
definitely not enough demand to merit the awkwardness of e.g.
Rope.repeat(n) inheriting a repeat returning a String.

Based on this data, I'd recommend providing one and only one method of this
type: String.repeat(int).  There's no real advantage to a static
repeat(char, int) method when the overwhelming majority of these are
constants: e.g. compare SomeUtilClass.repeat('*', n) versus "*".repeat(n).
Character.toString(c).repeat(n) isn't a bad workaround if you don't have a
constant char.  There also isn't much demand for dealing with the code
point case specially; the String.repeat(int) method seems like it'd handle
that just fine.

On Thu, Feb 15, 2018 at 11:44 AM Jim Laskey  wrote:

>
>
> > On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov 
> wrote:
> >
> > Hello!
> >
> > The link with the webrev returned 404, but I could find it at this
> location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
> >
> > A few minor comments:
> >
> > 1)
> >
> > This check:
> >
> > 2992 final long limit = (long)count * 2L;
> > 2993 if ((long)Integer.MAX_VALUE < limit) {
> >
> > can be possibly simplified as
> > if (count > Integer.MAX_VALUE - count) {
>
> Good.
>
> >
> > 2)
> > Should String repeat(final int codepoint, final int count) be optimized
> for codepoints that can be represented with a single char?
> >
> > E.g. like this:
> >
> > public static String repeat(final int codepoint, final int count) {
> >return Character.isBmpCodePoint(codepoint))
> >? repeat((char) codepoint, count)
> >: (new String(Character.toChars(codepoint))).repeat(count);
> > }
>
> Yes, avoid array allocation.
>
> >
> > 3)
> > Using long arithmetic can possibly be avoided in the common path of
> repeat(final int count):
> >
> > E.g. like this:
> >
> > if (count < 0) {
> > throw new IllegalArgumentException("count is negative, " +
> count);
> > } else if (count == 1) {
> > return this;
> > } else if (count == 0) {
> > return "";
> > }
> > final int len = value.length;
> > if (Integer.MAX_VALUE / count < len) {
> > throw new IllegalArgumentException(
> > "Resulting string exceeds maximum string length: " +
> ((long)len * (long)count));
> > }
> > final int limit = count * len;
>
> Good.
>
> Thank you.
>
> >
> > With kind regards,
> > Ivan
> >
> > On 2/15/18 9:20 AM, Jim Laskey wrote:
> >> This is a pre-CSR code review [1] for String repeat methods
> (Enhancement).
> >>
> >> The proposal is to introduce four new methods;
> >>
> >> 1. public String repeat(final int count)
> >> 2. public static String repeat(final char ch, final int count)
> >> 3. public static String repeat(final int codepoint, final int count)
> >> 4. public static String repeat(final CharSequence seq, final int count)
> >>
> >> For the sake of transparency, only 1 is necessary, 2-4 are convenience
> methods.
> >> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’,
> 10).
> >> 3 and 4 convert to String before calling 1.
> >>
> >> Performance runs with jmh (results as comment in [2]) show that these
> >> methods are significantly faster that StringBuilder equivalents.
> >>  - fewer memory allocations
> >>  - fewer char to byte array conversions
> >>  - faster pyramid replication vs O(N) copying
> >>
> >> I left StringBuilder out of scope. It falls under the category of
> >> Appendables#append with repeat. A much bigger project.
> >>
> >> All comments welcome. Especially around the need for convenience
> >> methods, the JavaDoc content and expanding the tests.
> >>
> >> — Jim
> >>
> >> [1] webrev:
> http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
> >> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
> >>
> >>
> >
> > --
> > With kind regards,
> > Ivan Gerasimov
> >
>
>

Re: RFR: 8197594 - String and character repeat

[Jim Laskey]

Thank you Roger.

> On Feb 15, 2018, at 4:38 PM, Roger Riggs  wrote:
> 
> Hi Jim,
> 
> Its cleaner to do the API (CSR) review before and without the implementation.
> It helps keep the issues separate.

This was on advice from a member of the core libs team. He can speak up if he 
wants.

> 
> Don't make statements about count == Integer.MAX_VALUE / 2.
> There is no point, unless it should throw IAE.
> 
> On 2/15/2018 3:16 PM, Jim Laskey wrote:
>>> On Feb 15, 2018, at 4:04 PM, Remi Forax  wrote:
>>> 
>>> I'm not sure we need 4, it's just a convenient method that may be slower 
>>> than if the user code calls toString() (because of profile pollution),
>>> so i'm not sure i pull it's own weight.
>>> 
>>> And about adding a default method into CharSequence, the default method 
>>> should return a CharSequence or String ?
>> If you mean each class returns an instance of its class, I think that 
>> overlaps Appendable..
> Beware thinking about default methods on interfaces; the dragons will get you.
> Recently, this was discussed in the context of Reader.transferTo.
>>> and what about the other implementations, AbstractStringBuilder and 
>>> CharBuffer at least ?
>> This falls into the Appendable.append( T t, int count) realm mentioned 
>> originally.
>> 
>> Long term this could be a goal, and maybe defaulting CharSequence#repeat 
>> returning a string would be shortsighted.
>> 
>> But, I think having instance String#repeat returning a CharSequence would 
>> limit its use (methods expecting strings.)
> There is no point in returning CharSequence, a String is a CharSequence and 
> can be used anywhere
> a CharSequence can.
> 
> $.02, Roger
> 
>> 
>> — Jim
>> 
>>> Rémi
>>> 
>>> - Mail original -
 De: "Jim Laskey" 
 À: "Brian Goetz" 
 Cc: "core-libs-dev" 
 Envoyé: Jeudi 15 Février 2018 18:34:19
 Objet: Re: RFR: 8197594 - String and character repeat
 Very reasonable approach.
 
 
> On Feb 15, 2018, at 1:31 PM, Brian Goetz  wrote:
> 
> I suggest merging 1 and 4 by making it an instance method on CS with a 
> default
> in CS and an override on string and string builder?
> 
> Sent from my MacBook Wheel
> 
>> On Feb 15, 2018, at 9:20 AM, Jim Laskey  wrote:
>> 
>> This is a pre-CSR code review [1] for String repeat methods 
>> (Enhancement).
>> 
>> The proposal is to introduce four new methods;
>> 
>> 1. public String repeat(final int count)
>> 2. public static String repeat(final char ch, final int count)
>> 3. public static String repeat(final int codepoint, final int count)
>> 4. public static String repeat(final CharSequence seq, final int count)
>> 
>> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>> methods.
>> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 
>> 10).
>> 3 and 4 convert to String before calling 1.
>> 
>> Performance runs with jmh (results as comment in [2]) show that these
>> methods are significantly faster that StringBuilder equivalents.
>> - fewer memory allocations
>> - fewer char to byte array conversions
>> - faster pyramid replication vs O(N) copying
>> 
>> I left StringBuilder out of scope. It falls under the category of
>> Appendables#append with repeat. A much bigger project.
>> 
>> All comments welcome. Especially around the need for convenience
>> methods, the JavaDoc content and expanding the tests.
>> 
>> — Jim
>> 
>> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>> 
> 

Re: RFR: 8197594 - String and character repeat

[Roger Riggs]

Hi Jim,

Its cleaner to do the API (CSR) review before and without the 
implementation.

It helps keep the issues separate.

Don't make statements about count == Integer.MAX_VALUE / 2.
There is no point, unless it should throw IAE.

On 2/15/2018 3:16 PM, Jim Laskey wrote:

On Feb 15, 2018, at 4:04 PM, Remi Forax  wrote:

I'm not sure we need 4, it's just a convenient method that may be slower than 
if the user code calls toString() (because of profile pollution),
so i'm not sure i pull it's own weight.

And about adding a default method into CharSequence, the default method should 
return a CharSequence or String ?

If you mean each class returns an instance of its class, I think that overlaps 
Appendable..
Beware thinking about default methods on interfaces; the dragons will 
get you.

Recently, this was discussed in the context of Reader.transferTo.

and what about the other implementations, AbstractStringBuilder and CharBuffer 
at least ?

This falls into the Appendable.append( T t, int count) realm mentioned 
originally.

Long term this could be a goal, and maybe defaulting CharSequence#repeat 
returning a string would be shortsighted.

But, I think having instance String#repeat returning a CharSequence would limit 
its use (methods expecting strings.)
There is no point in returning CharSequence, a String is a CharSequence 
and can be used anywhere

a CharSequence can.

$.02, Roger



— Jim


Rémi

- Mail original -

De: "Jim Laskey" 
À: "Brian Goetz" 
Cc: "core-libs-dev" 
Envoyé: Jeudi 15 Février 2018 18:34:19
Objet: Re: RFR: 8197594 - String and character repeat
Very reasonable approach.



On Feb 15, 2018, at 1:31 PM, Brian Goetz  wrote:

I suggest merging 1 and 4 by making it an instance method on CS with a default
in CS and an override on string and string builder?

Sent from my MacBook Wheel


On Feb 15, 2018, at 9:20 AM, Jim Laskey  wrote:

This is a pre-CSR code review [1] for String repeat methods (Enhancement).

The proposal is to introduce four new methods;

1. public String repeat(final int count)
2. public static String repeat(final char ch, final int count)
3. public static String repeat(final int codepoint, final int count)
4. public static String repeat(final CharSequence seq, final int count)

For the sake of transparency, only 1 is necessary, 2-4 are convenience methods.
In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
3 and 4 convert to String before calling 1.

Performance runs with jmh (results as comment in [2]) show that these
methods are significantly faster that StringBuilder equivalents.
- fewer memory allocations
- fewer char to byte array conversions
- faster pyramid replication vs O(N) copying

I left StringBuilder out of scope. It falls under the category of
Appendables#append with repeat. A much bigger project.

All comments welcome. Especially around the need for convenience
methods, the JavaDoc content and expanding the tests.

— Jim

[1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
[2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594

Re: RFR: 8197594 - String and character repeat

[forax]

- Mail original -
> De: "Jim Laskey" 
> À: "Remi Forax" 
> Cc: "Brian Goetz" , "core-libs-dev" 
> 
> Envoyé: Jeudi 15 Février 2018 21:16:48
> Objet: Re: RFR: 8197594 - String and character repeat

>> On Feb 15, 2018, at 4:04 PM, Remi Forax  wrote:
>> 
>> I'm not sure we need 4, it's just a convenient method that may be slower 
>> than if
>> the user code calls toString() (because of profile pollution),
>> so i'm not sure i pull it's own weight.
>> 
>> And about adding a default method into CharSequence, the default method 
>> should
>> return a CharSequence or String ?
> 
> If you mean each class returns an instance of its class, I think that overlaps
> Appendable..

yes, very true

> 
>> and what about the other implementations, AbstractStringBuilder and 
>> CharBuffer
>> at least ?
> 
> This falls into the Appendable.append( T t, int count) realm mentioned
> originally.
> 
> Long term this could be a goal, and maybe defaulting CharSequence#repeat
> returning a string would be shortsighted.
> 
> But, I think having instance String#repeat returning a CharSequence would 
> limit
> its use (methods expecting strings.)

yes, i think we should play safe here and only add repeat to String.

> 
> — Jim

Rémi

> 
>> 
>> Rémi
>> 
>> - Mail original -
>>> De: "Jim Laskey" 
>>> À: "Brian Goetz" 
>>> Cc: "core-libs-dev" 
>>> Envoyé: Jeudi 15 Février 2018 18:34:19
>>> Objet: Re: RFR: 8197594 - String and character repeat
>> 
>>> Very reasonable approach.
>>> 
>>> 
 On Feb 15, 2018, at 1:31 PM, Brian Goetz  wrote:
 
 I suggest merging 1 and 4 by making it an instance method on CS with a 
 default
 in CS and an override on string and string builder?
 
 Sent from my MacBook Wheel
 
> On Feb 15, 2018, at 9:20 AM, Jim Laskey  wrote:
> 
> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
> 
> The proposal is to introduce four new methods;
> 
> 1. public String repeat(final int count)
> 2. public static String repeat(final char ch, final int count)
> 3. public static String repeat(final int codepoint, final int count)
> 4. public static String repeat(final CharSequence seq, final int count)
> 
> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
> methods.
> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 
> 10).
> 3 and 4 convert to String before calling 1.
> 
> Performance runs with jmh (results as comment in [2]) show that these
> methods are significantly faster that StringBuilder equivalents.
> - fewer memory allocations
> - fewer char to byte array conversions
> - faster pyramid replication vs O(N) copying
> 
> I left StringBuilder out of scope. It falls under the category of
> Appendables#append with repeat. A much bigger project.
> 
> All comments welcome. Especially around the need for convenience
> methods, the JavaDoc content and expanding the tests.
> 
> — Jim
> 
> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[yumin qi]

Hi, Alan

  The real reason, for why File.getCanonicalPath() will fail after
"user.dir" property changed is, in File.java:

 public String getCanonicalPath() throws IOException {
if (isInvalid()) {
throw new IOException("Invalid file path");
}
return fs.canonicalize(fs.resolve(this));
}

It passed this File to FileSystem.resolve(File):

public String resolve(File f) {
if (isAbsolute(f)) return f.getPath();
return resolve(System.getProperty("user.dir"), f.getPath());
}
Note, here it get property of "user.dir" and passed the string directly
into resolve(String, String). Checked all the usage of this function in all
other places, they all normalize the string first then pass it to resolve.
Like:

File.java:

public File(String parent, String child) {
if (child == null) {
throw new NullPointerException();
}
if (parent != null) {
if (parent.equals("")) {
this.path = fs.resolve(fs.getDefaultParent(),
   fs.normalize(child));
} else {
this.path = fs.resolve(fs.normalize(parent),
   fs.normalize(child));
}
} else {
this.path = fs.normalize(child);
}
this.prefixLength = fs.prefixLength(this.path);
}

Since the property string contains non-normalized characters, it
crashed in native canonicalize.
I believe user.dir from the system is normalized, so it is OK but after
it is changed like "/home/a/b/c/", it crashed.

Now with using cached "user.dir", the problem is gone.
The ultimate guaranteed solution may be:

public UnixFileSystem() {
Properties props = GetPropertyAction.privilegedGetProperties();
slash = props.getProperty("file.separator").charAt(0);
colon = props.getProperty("path.separator").charAt(0);
javaHome = props.getProperty("java.home");
+userDir = normalize(props.getProperty("user.dir")); // is it
necessary?
}

 public String resolve(File f) {
 if (isAbsolute(f)) return f.getPath();-return
resolve(System.getProperty("user.dir"), f.getPath());+
SecurityManager sm = System.getSecurityManager();+if (sm !=
null) {+sm.checkPropertyAccess("user.dir");+}+
   return resolve(userDir, f.getPath());
 }


So the changes in resolve should be removed.

Since the bug is talking about the crash, the real reason is user.dir
should not be changed, how about changing description to
8194154: System property user.dir should not be changed.

 The test case renamed to: UserDirChangedTest.java   ?

Thanks
Yumin



On Thu, Feb 15, 2018 at 10:41 AM, yumin qi  wrote:

> There are two problems here, so become complex.
> 1) crash on parsing "//", which included in file path, on linux. This is
> fixed in UnixFileSystem.java in resolve function.
> 2) user.dir should be read only. This is fixed in both UnixFileSystem.java
> and WinNTFileSystem.java.
>
> The test case covers two of them.
> Should we handle them in two separate bugs?
>
> Yumin
>
>
> On Thu, Feb 15, 2018 at 10:00 AM, yumin qi  wrote:
>
>> OK, let's work on a suitable test case.
>>
>> Thanks
>> Yumin
>>
>> On Thu, Feb 15, 2018 at 9:41 AM, Alan Bateman 
>> wrote:
>>
>>> On 15/02/2018 17:25, yumin qi wrote:
>>>
 Alan,

   The real reason is if we do not change on resolve, the string passed
 into native canonicalize will be like "//./a/" which is not expected in
 native part. In native part, we resume that no more "//" in the path string
 (already normalized in java).

 If that is the case then the test doesn't match the issue we have been
>>> discussing on this thread. Would it be possible to create a test case for
>>> the issue that you are actually trying to address?
>>>
>>> -Alan
>>>
>>
>>
>

Re: RFR: 8197594 - String and character repeat

[Jim Laskey]

Thank you Remi.

Re final: Attila got into my head.

> On Feb 15, 2018, at 4:24 PM, Remi Forax  wrote:
> 
> - Mail original -
>> De: "Ivan Gerasimov" 
>> À: "Jim Laskey" , "core-libs-dev" 
>> 
>> Envoyé: Jeudi 15 Février 2018 20:36:44
>> Objet: Re: RFR: 8197594 - String and character repeat
> 
>> Hello!
>> 
>> The link with the webrev returned 404, but I could find it at this
>> location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
> 
> Jim,
> in static String repeat(char ch, int count) 
>  you can remove the 'else's because they are after a throw or a return as you 
> have done in the other methods.
> 
> in static String repeat(int codepoint, int count),
>  the parenthesis just after the return seems unnecessary,
> 
> in String repeat(int count)
>  at the end instead of testing isLatin1()  
>return new String(bytes, coder);
>  should be equivalent.
> 
> in replicate
>  you can return 'result' after the call to Arrays.fill, to signal to readers 
> that this is an optimization and that the primary code is below, it will also 
> avoid to shift the primary code to the right.
> 
> and nitpicking about the style, can you remove those pesky final, it's the 
> Nashorn style but not the JDK one :)
> 
> cheers,
> Rémi
> 
> 
>> On 2/15/18 9:20 AM, Jim Laskey wrote:
>>> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>>> 
>>> The proposal is to introduce four new methods;
>>> 
>>> 1. public String repeat(final int count)
>>> 2. public static String repeat(final char ch, final int count)
>>> 3. public static String repeat(final int codepoint, final int count)
>>> 4. public static String repeat(final CharSequence seq, final int count)
>>> 
>>> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>>> methods.
>>> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
>>> 3 and 4 convert to String before calling 1.
>>> 
>>> Performance runs with jmh (results as comment in [2]) show that these
>>> methods are significantly faster that StringBuilder equivalents.
>>>  - fewer memory allocations
>>>  - fewer char to byte array conversions
>>>  - faster pyramid replication vs O(N) copying
>>> 
>>> I left StringBuilder out of scope. It falls under the category of
>>> Appendables#append with repeat. A much bigger project.
>>> 
>>> All comments welcome. Especially around the need for convenience
>>> methods, the JavaDoc content and expanding the tests.
>>> 
>>> — Jim
>>> 
>>> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>>> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>>> 
>>> 
>> 
>> --
>> With kind regards,
>> Ivan Gerasimov

Re: RFR: 8197594 - String and character repeat

[Remi Forax]

- Mail original -
> De: "Ivan Gerasimov" 
> À: "Jim Laskey" , "core-libs-dev" 
> 
> Envoyé: Jeudi 15 Février 2018 20:36:44
> Objet: Re: RFR: 8197594 - String and character repeat

> Hello!
> 
> The link with the webrev returned 404, but I could find it at this
> location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/

Jim,
in static String repeat(char ch, int count) 
  you can remove the 'else's because they are after a throw or a return as you 
have done in the other methods.

in static String repeat(int codepoint, int count),
  the parenthesis just after the return seems unnecessary,

in String repeat(int count)
  at the end instead of testing isLatin1()  
return new String(bytes, coder);
  should be equivalent.

in replicate
  you can return 'result' after the call to Arrays.fill, to signal to readers 
that this is an optimization and that the primary code is below, it will also 
avoid to shift the primary code to the right.

and nitpicking about the style, can you remove those pesky final, it's the 
Nashorn style but not the JDK one :)

cheers,
Rémi


> On 2/15/18 9:20 AM, Jim Laskey wrote:
>> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>>
>> The proposal is to introduce four new methods;
>>
>> 1. public String repeat(final int count)
>> 2. public static String repeat(final char ch, final int count)
>> 3. public static String repeat(final int codepoint, final int count)
>> 4. public static String repeat(final CharSequence seq, final int count)
>>
>> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>> methods.
>> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
>> 3 and 4 convert to String before calling 1.
>>
>> Performance runs with jmh (results as comment in [2]) show that these
>> methods are significantly faster that StringBuilder equivalents.
>>   - fewer memory allocations
>>   - fewer char to byte array conversions
>>   - faster pyramid replication vs O(N) copying
>>
>> I left StringBuilder out of scope. It falls under the category of
>> Appendables#append with repeat. A much bigger project.
>>
>> All comments welcome. Especially around the need for convenience
>> methods, the JavaDoc content and expanding the tests.
>>
>> — Jim
>>
>> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>>
>>
> 
> --
> With kind regards,
> Ivan Gerasimov

Re: RFR: 8197594 - String and character repeat

[Jim Laskey]

> On Feb 15, 2018, at 4:04 PM, Remi Forax  wrote:
> 
> I'm not sure we need 4, it's just a convenient method that may be slower than 
> if the user code calls toString() (because of profile pollution),
> so i'm not sure i pull it's own weight.
> 
> And about adding a default method into CharSequence, the default method 
> should return a CharSequence or String ? 

If you mean each class returns an instance of its class, I think that overlaps 
Appendable..

> and what about the other implementations, AbstractStringBuilder and 
> CharBuffer at least ? 

This falls into the Appendable.append( T t, int count) realm mentioned 
originally.

Long term this could be a goal, and maybe defaulting CharSequence#repeat 
returning a string would be shortsighted.

But, I think having instance String#repeat returning a CharSequence would limit 
its use (methods expecting strings.)

— Jim

> 
> Rémi
> 
> - Mail original -
>> De: "Jim Laskey" 
>> À: "Brian Goetz" 
>> Cc: "core-libs-dev" 
>> Envoyé: Jeudi 15 Février 2018 18:34:19
>> Objet: Re: RFR: 8197594 - String and character repeat
> 
>> Very reasonable approach.
>> 
>> 
>>> On Feb 15, 2018, at 1:31 PM, Brian Goetz  wrote:
>>> 
>>> I suggest merging 1 and 4 by making it an instance method on CS with a 
>>> default
>>> in CS and an override on string and string builder?
>>> 
>>> Sent from my MacBook Wheel
>>> 
 On Feb 15, 2018, at 9:20 AM, Jim Laskey  wrote:
 
 This is a pre-CSR code review [1] for String repeat methods (Enhancement).
 
 The proposal is to introduce four new methods;
 
 1. public String repeat(final int count)
 2. public static String repeat(final char ch, final int count)
 3. public static String repeat(final int codepoint, final int count)
 4. public static String repeat(final CharSequence seq, final int count)
 
 For the sake of transparency, only 1 is necessary, 2-4 are convenience 
 methods.
 In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 
 10).
 3 and 4 convert to String before calling 1.
 
 Performance runs with jmh (results as comment in [2]) show that these
 methods are significantly faster that StringBuilder equivalents.
 - fewer memory allocations
 - fewer char to byte array conversions
 - faster pyramid replication vs O(N) copying
 
 I left StringBuilder out of scope. It falls under the category of
 Appendables#append with repeat. A much bigger project.
 
 All comments welcome. Especially around the need for convenience
 methods, the JavaDoc content and expanding the tests.
 
 — Jim
 
 [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
 [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
 

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[Alan Bateman]

On 15/02/2018 18:41, yumin qi wrote:

There are two problems here, so become complex.
1) crash on parsing "//", which included in file path, on linux. This 
is fixed in UnixFileSystem.java in resolve function.
2) user.dir should be read only. This is fixed in both 
UnixFileSystem.java and WinNTFileSystem.java.


The test case covers two of them.
Should we handle them in two separate bugs?

Assuming #2 is fixed and user.dir cannot be changed, do you still have 
#1? The current test just ensures that bad code changing user.dir in a 
running VM doesn't change getCanonicalPath. If there is more to #1 then 
I think it will need further tests.


-Alan

Re: RFR: 8197594 - String and character repeat

[Remi Forax]

I'm not sure we need 4, it's just a convenient method that may be slower than 
if the user code calls toString() (because of profile pollution),
so i'm not sure i pull it's own weight.

And about adding a default method into CharSequence, the default method should 
return a CharSequence or String ? and what about the other implementations, 
AbstractStringBuilder and CharBuffer at least ? 

Rémi

- Mail original -
> De: "Jim Laskey" 
> À: "Brian Goetz" 
> Cc: "core-libs-dev" 
> Envoyé: Jeudi 15 Février 2018 18:34:19
> Objet: Re: RFR: 8197594 - String and character repeat

> Very reasonable approach.
> 
> 
>> On Feb 15, 2018, at 1:31 PM, Brian Goetz  wrote:
>> 
>> I suggest merging 1 and 4 by making it an instance method on CS with a 
>> default
>> in CS and an override on string and string builder?
>> 
>> Sent from my MacBook Wheel
>> 
>>> On Feb 15, 2018, at 9:20 AM, Jim Laskey  wrote:
>>> 
>>> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>>> 
>>> The proposal is to introduce four new methods;
>>> 
>>> 1. public String repeat(final int count)
>>> 2. public static String repeat(final char ch, final int count)
>>> 3. public static String repeat(final int codepoint, final int count)
>>> 4. public static String repeat(final CharSequence seq, final int count)
>>> 
>>> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>>> methods.
>>> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
>>> 3 and 4 convert to String before calling 1.
>>> 
>>> Performance runs with jmh (results as comment in [2]) show that these
>>> methods are significantly faster that StringBuilder equivalents.
>>> - fewer memory allocations
>>> - fewer char to byte array conversions
>>> - faster pyramid replication vs O(N) copying
>>> 
>>> I left StringBuilder out of scope. It falls under the category of
>>> Appendables#append with repeat. A much bigger project.
>>> 
>>> All comments welcome. Especially around the need for convenience
>>> methods, the JavaDoc content and expanding the tests.
>>> 
>>> — Jim
>>> 
>>> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>>> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>>> 

Re: RFR: 8197594 - String and character repeat

[Jim Laskey]

> On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov  wrote:
> 
> Hello!
> 
> The link with the webrev returned 404, but I could find it at this location: 
> http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/
> 
> A few minor comments:
> 
> 1)
> 
> This check:
> 
> 2992 final long limit = (long)count * 2L;
> 2993 if ((long)Integer.MAX_VALUE < limit) {
> 
> can be possibly simplified as
> if (count > Integer.MAX_VALUE - count) {

Good.

> 
> 2)
> Should String repeat(final int codepoint, final int count) be optimized for 
> codepoints that can be represented with a single char?
> 
> E.g. like this:
> 
> public static String repeat(final int codepoint, final int count) {
>return Character.isBmpCodePoint(codepoint))
>? repeat((char) codepoint, count)
>: (new String(Character.toChars(codepoint))).repeat(count);
> }

Yes, avoid array allocation.

> 
> 3)
> Using long arithmetic can possibly be avoided in the common path of 
> repeat(final int count):
> 
> E.g. like this:
> 
> if (count < 0) {
> throw new IllegalArgumentException("count is negative, " + count);
> } else if (count == 1) {
> return this;
> } else if (count == 0) {
> return "";
> }
> final int len = value.length;
> if (Integer.MAX_VALUE / count < len) {
> throw new IllegalArgumentException(
> "Resulting string exceeds maximum string length: " + 
> ((long)len * (long)count));
> }
> final int limit = count * len;

Good.

Thank you.

> 
> With kind regards,
> Ivan
> 
> On 2/15/18 9:20 AM, Jim Laskey wrote:
>> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>> 
>> The proposal is to introduce four new methods;
>> 
>> 1. public String repeat(final int count)
>> 2. public static String repeat(final char ch, final int count)
>> 3. public static String repeat(final int codepoint, final int count)
>> 4. public static String repeat(final CharSequence seq, final int count)
>> 
>> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>> methods.
>> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
>> 3 and 4 convert to String before calling 1.
>> 
>> Performance runs with jmh (results as comment in [2]) show that these
>> methods are significantly faster that StringBuilder equivalents.
>>  - fewer memory allocations
>>  - fewer char to byte array conversions
>>  - faster pyramid replication vs O(N) copying
>> 
>> I left StringBuilder out of scope. It falls under the category of
>> Appendables#append with repeat. A much bigger project.
>> 
>> All comments welcome. Especially around the need for convenience
>> methods, the JavaDoc content and expanding the tests.
>> 
>> — Jim
>> 
>> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>> 
>> 
> 
> -- 
> With kind regards,
> Ivan Gerasimov
> 

Re: RFR: 8197594 - String and character repeat

[Ivan Gerasimov]

Hello!

The link with the webrev returned 404, but I could find it at this 
location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/


A few minor comments:

1)

This check:

2992 final long limit = (long)count * 2L;
2993 if ((long)Integer.MAX_VALUE < limit) {

can be possibly simplified as
if (count > Integer.MAX_VALUE - count) {

2)
Should String repeat(final int codepoint, final int count) be optimized 
for codepoints that can be represented with a single char?


E.g. like this:

public static String repeat(final int codepoint, final int count) {
return Character.isBmpCodePoint(codepoint))
? repeat((char) codepoint, count)
: (new String(Character.toChars(codepoint))).repeat(count);
}

3)
Using long arithmetic can possibly be avoided in the common path of 
repeat(final int count):


E.g. like this:

 if (count < 0) {
 throw new IllegalArgumentException("count is negative, " + 
count);

 } else if (count == 1) {
 return this;
 } else if (count == 0) {
 return "";
}
 final int len = value.length;
 if (Integer.MAX_VALUE / count < len) {
 throw new IllegalArgumentException(
 "Resulting string exceeds maximum string length: " 
+ ((long)len * (long)count));

 }
 final int limit = count * len;

With kind regards,
Ivan

On 2/15/18 9:20 AM, Jim Laskey wrote:

This is a pre-CSR code review [1] for String repeat methods (Enhancement).

The proposal is to introduce four new methods;

1. public String repeat(final int count)
2. public static String repeat(final char ch, final int count)
3. public static String repeat(final int codepoint, final int count)
4. public static String repeat(final CharSequence seq, final int count)

For the sake of transparency, only 1 is necessary, 2-4 are convenience methods.
In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
3 and 4 convert to String before calling 1.

Performance runs with jmh (results as comment in [2]) show that these
methods are significantly faster that StringBuilder equivalents.
  - fewer memory allocations
  - fewer char to byte array conversions
  - faster pyramid replication vs O(N) copying

I left StringBuilder out of scope. It falls under the category of
Appendables#append with repeat. A much bigger project.

All comments welcome. Especially around the need for convenience
methods, the JavaDoc content and expanding the tests.

— Jim

[1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
[2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594




--
With kind regards,
Ivan Gerasimov

Re: RFR: 8041626: [Event Request] Shutdown reason

[mandy chung]

Hi Robin,

Do you want a shutdown event for every call to Runtime::exit regardless 
where it is in the shutdown sequence?  or do you expect to get an event 
of the first thread calling JVM_Halt?  or the first thread starting the 
shutdown sequence but it may not be the thread halting?


Mandy

On 2/15/18 5:35 AM, Robin Westberg wrote:

Hi again,

Here’s the (hopefully) final version:

Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ 

Incremental: 
http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ 



Best regards,
Robin

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[yumin qi]

There are two problems here, so become complex.
1) crash on parsing "//", which included in file path, on linux. This is
fixed in UnixFileSystem.java in resolve function.
2) user.dir should be read only. This is fixed in both UnixFileSystem.java
and WinNTFileSystem.java.

The test case covers two of them.
Should we handle them in two separate bugs?

Yumin


On Thu, Feb 15, 2018 at 10:00 AM, yumin qi  wrote:

> OK, let's work on a suitable test case.
>
> Thanks
> Yumin
>
> On Thu, Feb 15, 2018 at 9:41 AM, Alan Bateman 
> wrote:
>
>> On 15/02/2018 17:25, yumin qi wrote:
>>
>>> Alan,
>>>
>>>   The real reason is if we do not change on resolve, the string passed
>>> into native canonicalize will be like "//./a/" which is not expected in
>>> native part. In native part, we resume that no more "//" in the path string
>>> (already normalized in java).
>>>
>>> If that is the case then the test doesn't match the issue we have been
>> discussing on this thread. Would it be possible to create a test case for
>> the issue that you are actually trying to address?
>>
>> -Alan
>>
>
>

Re: JEP [DRAFT]: Container aware Java

[Bob Vandette]

> On Feb 15, 2018, at 1:30 PM, kedar mhaswade  wrote:
> 
> This appears useful.
> 
> Will Runtime.getRuntime().availableProcessors() return the processors 
> available to the container after this integration, or will a new API be 
> provided? I have seen thread pools being misconfigured (e.g. it returns the 
> number of processors available on the host which is far more than that of 
> processors allotted by cgroups to the container) because of the confusion (or 
> because of just using the non-container-aware code).

I fixed your above problem in JDK 10 
(https://bugs.openjdk.java.net/browse/JDK-8146115 
).

This work exports much more detail about the container to monitoring tools.

Bob.

> 
> Regards,
> Kedar
> 
> On Thu, Feb 15, 2018 at 10:04 AM, Bob Vandette  > wrote:
> 
> > On Feb 15, 2018, at 12:52 PM, Volker Simonis  > > wrote:
> >
> > Sounds cool!
> >
> > Is this JEP only about providing the corresponding API or also about
> > using it internally (within HotSpot and class library) to better adopt
> > to environment the JVM is running in?
> 
> Thanks Volker.
> 
> I integrated JDK-8146115 into JDK 10 which allows Hotspot to adapt to the
> container it’s running in.  The configuration that is examined includes 
> memory limits,
> cpu count including cpu shares and quotas.
> 
> This JEP’s main focus is to provide an internal API that can be used by JDK 
> tools
> such as JFR and JMX to export container statistics and configuration data.  
> This JEP
> does not include the JFR and JMX implementations.  They will hopefully follow 
> shortly after.
> 
> Bob.
> 
> >
> > Either way, looking forward to see (and test) the first implementation bits!
> >
> > Regards,
> > Volker
> >
> >
> > On Thu, Feb 15, 2018 at 6:07 PM, Bob Vandette  > > wrote:
> >> I’d like to re-propose the following JEP that will enhance the Java 
> >> runtime to be more container aware.
> >> This will add an Internal Java API that will provide container specific 
> >> statistics.  Some of the initial goals
> >> of the previous JEP proposal has been integrated into JDK 10 under an RFE 
> >> (JDK-8146115).
> >> This JEP is now focused on providing a Java API that exports Container 
> >> runtime configuration and metrics.
> >>
> >> Since the scope of this JEP have changed, I’m re-submitting it for comment 
> >> and endorsement.
> >>
> >>
> >> JEP Issue:
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8182070 
> >> 
> >>
> >> Here’s a Text dump of the JEP contents for your convenience:
> >>
> >> Summary
> >> ---
> >>
> >> Container aware Java runtime
> >>
> >> Goals
> >> -
> >>
> >> Provide an internal API that can be used to extract container specific 
> >> configuration and runtime statistics.  This JEP will only support Docker 
> >> on Linux-x64 although the design should be flexible enough to allow 
> >> support for other platforms and container technologies.  The initial focus 
> >> will be on Linux cgroups technology so that we will be able to easily 
> >> support other container technologies running on Linux in addition to 
> >> Docker.
> >>
> >> Non-Goals
> >> -
> >>
> >> It is not a goal of this JEP to support any platform other than Docker 
> >> container technology running on Linux x64.
> >>
> >> Success Metrics
> >> ---
> >>
> >> Success will be measured by the improvement in information that will be 
> >> available to tools which visualize resource usage of containers that are 
> >> running Java processes.
> >>
> >> Motivation
> >> --
> >>
> >> Container technology is becoming more and more prevalent in Cloud based 
> >> applications.  The Cloud Serverless application programming model 
> >> motivates developers to split large monolithic applications into 100s of 
> >> smaller pieces each running in thier own container.  This move increases 
> >> the importance of the observability of each running container process.  
> >> Adding the proposed set of APIs will allow more details related to each 
> >> container process to be made available to external tools thereby improving 
> >> the observability.
> >>
> >> Description
> >> ---
> >>
> >> This enhancement will be made up of the following work items:
> >>
> >> A. Detecting if Java is running in a container.
> >>
> >> The Java runtime, as well as any tests that we might write for this 
> >> feature, will need to be able to detect that the current Java process is 
> >> running in a container.  A new API will be made available for this purpose.
> >>
> >> B. Exposing container resource limits, configuration and runtime 
> >> statistics.
> >>
> >> There are several configuration options and limits that can be imposed 
> >> upon a running 

Re: JEP [DRAFT]: Container aware Java

[kedar mhaswade]

This appears useful.

Will Runtime.getRuntime().availableProcessors() return the processors
available to the container after this integration, or will a new API be
provided? I have seen thread pools being misconfigured (e.g. it returns the
number of processors available on the host which is far more than that of
processors allotted by cgroups to the container) because of the confusion
(or because of just using the non-container-aware code).

Regards,
Kedar

On Thu, Feb 15, 2018 at 10:04 AM, Bob Vandette 
wrote:

>
> > On Feb 15, 2018, at 12:52 PM, Volker Simonis 
> wrote:
> >
> > Sounds cool!
> >
> > Is this JEP only about providing the corresponding API or also about
> > using it internally (within HotSpot and class library) to better adopt
> > to environment the JVM is running in?
>
> Thanks Volker.
>
> I integrated JDK-8146115 into JDK 10 which allows Hotspot to adapt to the
> container it’s running in.  The configuration that is examined includes
> memory limits,
> cpu count including cpu shares and quotas.
>
> This JEP’s main focus is to provide an internal API that can be used by
> JDK tools
> such as JFR and JMX to export container statistics and configuration
> data.  This JEP
> does not include the JFR and JMX implementations.  They will hopefully
> follow shortly after.
>
> Bob.
>
> >
> > Either way, looking forward to see (and test) the first implementation
> bits!
> >
> > Regards,
> > Volker
> >
> >
> > On Thu, Feb 15, 2018 at 6:07 PM, Bob Vandette 
> wrote:
> >> I’d like to re-propose the following JEP that will enhance the Java
> runtime to be more container aware.
> >> This will add an Internal Java API that will provide container specific
> statistics.  Some of the initial goals
> >> of the previous JEP proposal has been integrated into JDK 10 under an
> RFE (JDK-8146115).
> >> This JEP is now focused on providing a Java API that exports Container
> runtime configuration and metrics.
> >>
> >> Since the scope of this JEP have changed, I’m re-submitting it for
> comment and endorsement.
> >>
> >>
> >> JEP Issue:
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8182070
> >>
> >> Here’s a Text dump of the JEP contents for your convenience:
> >>
> >> Summary
> >> ---
> >>
> >> Container aware Java runtime
> >>
> >> Goals
> >> -
> >>
> >> Provide an internal API that can be used to extract container specific
> configuration and runtime statistics.  This JEP will only support Docker on
> Linux-x64 although the design should be flexible enough to allow support
> for other platforms and container technologies.  The initial focus will be
> on Linux cgroups technology so that we will be able to easily support other
> container technologies running on Linux in addition to Docker.
> >>
> >> Non-Goals
> >> -
> >>
> >> It is not a goal of this JEP to support any platform other than Docker
> container technology running on Linux x64.
> >>
> >> Success Metrics
> >> ---
> >>
> >> Success will be measured by the improvement in information that will be
> available to tools which visualize resource usage of containers that are
> running Java processes.
> >>
> >> Motivation
> >> --
> >>
> >> Container technology is becoming more and more prevalent in Cloud based
> applications.  The Cloud Serverless application programming model motivates
> developers to split large monolithic applications into 100s of smaller
> pieces each running in thier own container.  This move increases the
> importance of the observability of each running container process.  Adding
> the proposed set of APIs will allow more details related to each container
> process to be made available to external tools thereby improving the
> observability.
> >>
> >> Description
> >> ---
> >>
> >> This enhancement will be made up of the following work items:
> >>
> >> A. Detecting if Java is running in a container.
> >>
> >> The Java runtime, as well as any tests that we might write for this
> feature, will need to be able to detect that the current Java process is
> running in a container.  A new API will be made available for this purpose.
> >>
> >> B. Exposing container resource limits, configuration and runtime
> statistics.
> >>
> >> There are several configuration options and limits that can be imposed
> upon a running container.  Not all of these
> >> are important to a running Java process.  We clearly want to be able to
> detect how many CPUs have been allocated to our process along with the
> maximum amount of memory that the process has been allocated but there are
> other options that we might want to base runtime decisions on.
> >>
> >> In addition, since Container typically impose limits on system
> resources, they also provide the ability to easily access the amount of
> consumption of these resources.  The goal is to provide this information in
> addition to the configuration data.
> >>
> >> I propose adding a new 

Re: JEP [DRAFT]: Container aware Java

[Bob Vandette]

> On Feb 15, 2018, at 12:52 PM, Volker Simonis  wrote:
> 
> Sounds cool!
> 
> Is this JEP only about providing the corresponding API or also about
> using it internally (within HotSpot and class library) to better adopt
> to environment the JVM is running in?

Thanks Volker.

I integrated JDK-8146115 into JDK 10 which allows Hotspot to adapt to the
container it’s running in.  The configuration that is examined includes memory 
limits,
cpu count including cpu shares and quotas. 

This JEP’s main focus is to provide an internal API that can be used by JDK 
tools
such as JFR and JMX to export container statistics and configuration data.  
This JEP
does not include the JFR and JMX implementations.  They will hopefully follow 
shortly after.

Bob.

> 
> Either way, looking forward to see (and test) the first implementation bits!
> 
> Regards,
> Volker
> 
> 
> On Thu, Feb 15, 2018 at 6:07 PM, Bob Vandette  wrote:
>> I’d like to re-propose the following JEP that will enhance the Java runtime 
>> to be more container aware.
>> This will add an Internal Java API that will provide container specific 
>> statistics.  Some of the initial goals
>> of the previous JEP proposal has been integrated into JDK 10 under an RFE 
>> (JDK-8146115).
>> This JEP is now focused on providing a Java API that exports Container 
>> runtime configuration and metrics.
>> 
>> Since the scope of this JEP have changed, I’m re-submitting it for comment 
>> and endorsement.
>> 
>> 
>> JEP Issue:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8182070
>> 
>> Here’s a Text dump of the JEP contents for your convenience:
>> 
>> Summary
>> ---
>> 
>> Container aware Java runtime
>> 
>> Goals
>> -
>> 
>> Provide an internal API that can be used to extract container specific 
>> configuration and runtime statistics.  This JEP will only support Docker on 
>> Linux-x64 although the design should be flexible enough to allow support for 
>> other platforms and container technologies.  The initial focus will be on 
>> Linux cgroups technology so that we will be able to easily support other 
>> container technologies running on Linux in addition to Docker.
>> 
>> Non-Goals
>> -
>> 
>> It is not a goal of this JEP to support any platform other than Docker 
>> container technology running on Linux x64.
>> 
>> Success Metrics
>> ---
>> 
>> Success will be measured by the improvement in information that will be 
>> available to tools which visualize resource usage of containers that are 
>> running Java processes.
>> 
>> Motivation
>> --
>> 
>> Container technology is becoming more and more prevalent in Cloud based 
>> applications.  The Cloud Serverless application programming model motivates 
>> developers to split large monolithic applications into 100s of smaller 
>> pieces each running in thier own container.  This move increases the 
>> importance of the observability of each running container process.  Adding 
>> the proposed set of APIs will allow more details related to each container 
>> process to be made available to external tools thereby improving the 
>> observability.
>> 
>> Description
>> ---
>> 
>> This enhancement will be made up of the following work items:
>> 
>> A. Detecting if Java is running in a container.
>> 
>> The Java runtime, as well as any tests that we might write for this feature, 
>> will need to be able to detect that the current Java process is running in a 
>> container.  A new API will be made available for this purpose.
>> 
>> B. Exposing container resource limits, configuration and runtime statistics.
>> 
>> There are several configuration options and limits that can be imposed upon 
>> a running container.  Not all of these
>> are important to a running Java process.  We clearly want to be able to 
>> detect how many CPUs have been allocated to our process along with the 
>> maximum amount of memory that the process has been allocated but there are 
>> other options that we might want to base runtime decisions on.
>> 
>> In addition, since Container typically impose limits on system resources, 
>> they also provide the ability to easily access the amount of consumption of 
>> these resources.  The goal is to provide this information in addition to the 
>> configuration data.
>> 
>> I propose adding a new jdk.internal.Platform class that will allow access to 
>> this information.
>> 
>> Here are some of the types of configuration and consumption statistics that 
>> would be made available:
>> 
>>isContainerized
>>Memory Limit
>>Total Memory Limit
>>Soft Memory Limit
>>Max Memory Usage
>>Current Memory Usage
>>Maximum Kernel Memory
>>CPU Shares
>>CPU Period
>>CPU Quota
>>Number of CPUs
>>CPU Sets
>>CPU Set Memory Nodes
>>CPU Usage
>>CPU Usage Per CPU
>>Block I/O Weight
>>Block I/O Device Weight
>>Device I/O Read Rate
>>Device I/O Write Rate
>>OOM 

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[yumin qi]

OK, let's work on a suitable test case.

Thanks
Yumin

On Thu, Feb 15, 2018 at 9:41 AM, Alan Bateman 
wrote:

> On 15/02/2018 17:25, yumin qi wrote:
>
>> Alan,
>>
>>   The real reason is if we do not change on resolve, the string passed
>> into native canonicalize will be like "//./a/" which is not expected in
>> native part. In native part, we resume that no more "//" in the path string
>> (already normalized in java).
>>
>> If that is the case then the test doesn't match the issue we have been
> discussing on this thread. Would it be possible to create a test case for
> the issue that you are actually trying to address?
>
> -Alan
>

Re: JEP [DRAFT]: Container aware Java

[Volker Simonis]

Sounds cool!

Is this JEP only about providing the corresponding API or also about
using it internally (within HotSpot and class library) to better adopt
to environment the JVM is running in?

Either way, looking forward to see (and test) the first implementation bits!

Regards,
Volker


On Thu, Feb 15, 2018 at 6:07 PM, Bob Vandette  wrote:
> I’d like to re-propose the following JEP that will enhance the Java runtime 
> to be more container aware.
> This will add an Internal Java API that will provide container specific 
> statistics.  Some of the initial goals
> of the previous JEP proposal has been integrated into JDK 10 under an RFE 
> (JDK-8146115).
> This JEP is now focused on providing a Java API that exports Container 
> runtime configuration and metrics.
>
> Since the scope of this JEP have changed, I’m re-submitting it for comment 
> and endorsement.
>
>
> JEP Issue:
>
> https://bugs.openjdk.java.net/browse/JDK-8182070
>
> Here’s a Text dump of the JEP contents for your convenience:
>
> Summary
> ---
>
> Container aware Java runtime
>
> Goals
> -
>
> Provide an internal API that can be used to extract container specific 
> configuration and runtime statistics.  This JEP will only support Docker on 
> Linux-x64 although the design should be flexible enough to allow support for 
> other platforms and container technologies.  The initial focus will be on 
> Linux cgroups technology so that we will be able to easily support other 
> container technologies running on Linux in addition to Docker.
>
> Non-Goals
> -
>
> It is not a goal of this JEP to support any platform other than Docker 
> container technology running on Linux x64.
>
> Success Metrics
> ---
>
> Success will be measured by the improvement in information that will be 
> available to tools which visualize resource usage of containers that are 
> running Java processes.
>
> Motivation
> --
>
> Container technology is becoming more and more prevalent in Cloud based 
> applications.  The Cloud Serverless application programming model motivates 
> developers to split large monolithic applications into 100s of smaller pieces 
> each running in thier own container.  This move increases the importance of 
> the observability of each running container process.  Adding the proposed set 
> of APIs will allow more details related to each container process to be made 
> available to external tools thereby improving the observability.
>
> Description
> ---
>
> This enhancement will be made up of the following work items:
>
> A. Detecting if Java is running in a container.
>
> The Java runtime, as well as any tests that we might write for this feature, 
> will need to be able to detect that the current Java process is running in a 
> container.  A new API will be made available for this purpose.
>
> B. Exposing container resource limits, configuration and runtime statistics.
>
> There are several configuration options and limits that can be imposed upon a 
> running container.  Not all of these
> are important to a running Java process.  We clearly want to be able to 
> detect how many CPUs have been allocated to our process along with the 
> maximum amount of memory that the process has been allocated but there are 
> other options that we might want to base runtime decisions on.
>
> In addition, since Container typically impose limits on system resources, 
> they also provide the ability to easily access the amount of consumption of 
> these resources.  The goal is to provide this information in addition to the 
> configuration data.
>
> I propose adding a new jdk.internal.Platform class that will allow access to 
> this information.
>
> Here are some of the types of configuration and consumption statistics that 
> would be made available:
>
> isContainerized
> Memory Limit
> Total Memory Limit
> Soft Memory Limit
> Max Memory Usage
> Current Memory Usage
> Maximum Kernel Memory
> CPU Shares
> CPU Period
> CPU Quota
> Number of CPUs
> CPU Sets
> CPU Set Memory Nodes
> CPU Usage
> CPU Usage Per CPU
> Block I/O Weight
> Block I/O Device Weight
> Device I/O Read Rate
> Device I/O Write Rate
> OOM Kill Enabled
> OOM Score Adjustment
> Memory Swappiness
> Shared Memory Size
>
> Alternatives
> 
>
> There are a few existing tools available to extract some of the same 
> container statistics.  These tools could be used instead.  The benefit of 
> providing a core Java internal API is that this information can be expose by 
> current Java serviceability tools such as JMX and JFR along side other JVM 
> specific information.
>
> Testing
> ---
>
> Docker/container specific tests should be added in order to validate the 
> functionality being provided with this JEP.
>
> Risks and Assumptions
> -
>
> Docker is currently based on cgroups v1. Cgroups v2 is also available 

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[Alan Bateman]

On 15/02/2018 17:25, yumin qi wrote:

Alan,

  The real reason is if we do not change on resolve, the string passed 
into native canonicalize will be like "//./a/" which is not expected 
in native part. In native part, we resume that no more "//" in the 
path string (already normalized in java).


If that is the case then the test doesn't match the issue we have been 
discussing on this thread. Would it be possible to create a test case 
for the issue that you are actually trying to address?


-Alan

Re: RFR: 8197594 - String and character repeat

[Jim Laskey]

Very reasonable approach.


> On Feb 15, 2018, at 1:31 PM, Brian Goetz  wrote:
> 
> I suggest merging 1 and 4 by making it an instance method on CS with a 
> default in CS and an override on string and string builder?
> 
> Sent from my MacBook Wheel
> 
>> On Feb 15, 2018, at 9:20 AM, Jim Laskey  wrote:
>> 
>> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
>> 
>> The proposal is to introduce four new methods;
>> 
>> 1. public String repeat(final int count)
>> 2. public static String repeat(final char ch, final int count)
>> 3. public static String repeat(final int codepoint, final int count)
>> 4. public static String repeat(final CharSequence seq, final int count)
>> 
>> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
>> methods.
>> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
>> 3 and 4 convert to String before calling 1.
>> 
>> Performance runs with jmh (results as comment in [2]) show that these
>> methods are significantly faster that StringBuilder equivalents.
>> - fewer memory allocations
>> - fewer char to byte array conversions
>> - faster pyramid replication vs O(N) copying
>> 
>> I left StringBuilder out of scope. It falls under the category of
>> Appendables#append with repeat. A much bigger project.
>> 
>> All comments welcome. Especially around the need for convenience
>> methods, the JavaDoc content and expanding the tests.
>> 
>> — Jim
>> 
>> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
>> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
>> 
> 

Re: RFR: 8197594 - String and character repeat

[Brian Goetz]

I suggest merging 1 and 4 by making it an instance method on CS with a default 
in CS and an override on string and string builder?

Sent from my MacBook Wheel

> On Feb 15, 2018, at 9:20 AM, Jim Laskey  wrote:
> 
> This is a pre-CSR code review [1] for String repeat methods (Enhancement).
> 
> The proposal is to introduce four new methods;
> 
> 1. public String repeat(final int count)
> 2. public static String repeat(final char ch, final int count)
> 3. public static String repeat(final int codepoint, final int count)
> 4. public static String repeat(final CharSequence seq, final int count)
> 
> For the sake of transparency, only 1 is necessary, 2-4 are convenience 
> methods.
> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
> 3 and 4 convert to String before calling 1.
> 
> Performance runs with jmh (results as comment in [2]) show that these
> methods are significantly faster that StringBuilder equivalents.
> - fewer memory allocations
> - fewer char to byte array conversions
> - faster pyramid replication vs O(N) copying
> 
> I left StringBuilder out of scope. It falls under the category of
> Appendables#append with repeat. A much bigger project.
> 
> All comments welcome. Especially around the need for convenience
> methods, the JavaDoc content and expanding the tests.
> 
> — Jim
> 
> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594
> 

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[yumin qi]

Alan,

  The real reason is if we do not change on resolve, the string passed into
native canonicalize will be like "//./a/" which is not expected in native
part. In native part, we resume that no more "//" in the path string
(already normalized in java).
   We can fix this by  either:
   1) in java part, like the webrev here
or
2) in native canonicalize_md.c as webrev0.
   If the test run on Windows, should it run in unix like shell on Windows?
The separator "/" is not for Windows.

 Thanks
  Yumin



On Thu, Feb 15, 2018 at 12:23 AM, Alan Bateman 
wrote:

> On 14/02/2018 23:52, yumin qi wrote:
>
>> Brian,
>>
>>   Updated using RuntimeException, which will not need -ea so removed.
>> http://cr.openjdk.java.net/~minqi/8194154/webrev1/ <
>> http://cr.openjdk.java.net/%7Eminqi/8194154/webrev1/>
>>
>> Can you explain why resolve has been changed to call normalize
> parent+child? I can't help thinking there is something else going on if
> this change is needed.
>
> I see the mails about the test so I'll skip that except to say that it
> should not need "require". The test should be able to run on all platforms.
>
> -Alan
>
>

RFR: 8193660 Check SOURCE line in "release" file for closedjdk

[Randy Crihfield]

I need to revise the resource file test created for JDK-8192837

The new bug is https://bugs.openjdk.java.net/browse/JDK-8193660

The webrev is located at 
http://cr.openjdk.java.net/~shurailine/8193660/webrev.00/ 



Any comments/suggestions are welcome, also I will need a sponsor for it 
at the end…


Randy

RFR: 8197594 - String and character repeat

[Jim Laskey]

This is a pre-CSR code review [1] for String repeat methods (Enhancement).

The proposal is to introduce four new methods;

1. public String repeat(final int count)
2. public static String repeat(final char ch, final int count)
3. public static String repeat(final int codepoint, final int count)
4. public static String repeat(final CharSequence seq, final int count)

For the sake of transparency, only 1 is necessary, 2-4 are convenience methods.
In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, 10).
3 and 4 convert to String before calling 1.

Performance runs with jmh (results as comment in [2]) show that these
methods are significantly faster that StringBuilder equivalents.
 - fewer memory allocations
 - fewer char to byte array conversions
 - faster pyramid replication vs O(N) copying

I left StringBuilder out of scope. It falls under the category of
Appendables#append with repeat. A much bigger project.

All comments welcome. Especially around the need for convenience
methods, the JavaDoc content and expanding the tests.

— Jim

[1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00
[2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594

JEP [DRAFT]: Container aware Java

[Bob Vandette]

I’d like to re-propose the following JEP that will enhance the Java runtime to 
be more container aware.  
This will add an Internal Java API that will provide container specific 
statistics.  Some of the initial goals
of the previous JEP proposal has been integrated into JDK 10 under an RFE 
(JDK-8146115).
This JEP is now focused on providing a Java API that exports Container runtime 
configuration and metrics.

Since the scope of this JEP have changed, I’m re-submitting it for comment and 
endorsement.


JEP Issue:

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

Here’s a Text dump of the JEP contents for your convenience:

Summary
---

Container aware Java runtime

Goals
-

Provide an internal API that can be used to extract container specific 
configuration and runtime statistics.  This JEP will only support Docker on 
Linux-x64 although the design should be flexible enough to allow support for 
other platforms and container technologies.  The initial focus will be on Linux 
cgroups technology so that we will be able to easily support other container 
technologies running on Linux in addition to Docker.

Non-Goals
-

It is not a goal of this JEP to support any platform other than Docker 
container technology running on Linux x64.

Success Metrics
---

Success will be measured by the improvement in information that will be 
available to tools which visualize resource usage of containers that are 
running Java processes.

Motivation
--

Container technology is becoming more and more prevalent in Cloud based 
applications.  The Cloud Serverless application programming model motivates 
developers to split large monolithic applications into 100s of smaller pieces 
each running in thier own container.  This move increases the importance of the 
observability of each running container process.  Adding the proposed set of 
APIs will allow more details related to each container process to be made 
available to external tools thereby improving the observability.

Description
---

This enhancement will be made up of the following work items:

A. Detecting if Java is running in a container.

The Java runtime, as well as any tests that we might write for this feature, 
will need to be able to detect that the current Java process is running in a 
container.  A new API will be made available for this purpose.

B. Exposing container resource limits, configuration and runtime statistics.

There are several configuration options and limits that can be imposed upon a 
running container.  Not all of these
are important to a running Java process.  We clearly want to be able to detect 
how many CPUs have been allocated to our process along with the maximum amount 
of memory that the process has been allocated but there are other options that 
we might want to base runtime decisions on.

In addition, since Container typically impose limits on system resources, they 
also provide the ability to easily access the amount of consumption of these 
resources.  The goal is to provide this information in addition to the 
configuration data.

I propose adding a new jdk.internal.Platform class that will allow access to 
this information.  

Here are some of the types of configuration and consumption statistics that 
would be made available:

isContainerized
Memory Limit 
Total Memory Limit
Soft Memory Limit
Max Memory Usage
Current Memory Usage 
Maximum Kernel Memory
CPU Shares
CPU Period
CPU Quota
Number of CPUs
CPU Sets
CPU Set Memory Nodes
CPU Usage
CPU Usage Per CPU
Block I/O Weight
Block I/O Device Weight 
Device I/O Read Rate
Device I/O Write Rate
OOM Kill Enabled
OOM Score Adjustment
Memory Swappiness
Shared Memory Size

Alternatives


There are a few existing tools available to extract some of the same container 
statistics.  These tools could be used instead.  The benefit of providing a 
core Java internal API is that this information can be expose by current Java 
serviceability tools such as JMX and JFR along side other JVM specific 
information.

Testing
---

Docker/container specific tests should be added in order to validate the 
functionality being provided with this JEP.

Risks and Assumptions
-

Docker is currently based on cgroups v1. Cgroups v2 is also available but is 
incomplete and not yet supported by Docker. It's possible that v2 could replace 
v1 in an incompatible way rendering this work unusable until it is upgraded.

Other alternative container technologies based on hypervisors are being 
developed that could replace the use of cgroups for container isloation.

Dependencies
---

None at this time.

Re: RFR: 8197812: (ref) Data race in Finalizer

[Martin Buchholz]

On Wed, Feb 14, 2018 at 11:47 PM, Peter Levart 
wrote:

>
> Although not strictly necessary for correctness, for the time being, it
> would be nice to be consistent in marking the Finalizer object "already
> finalized" in both places. Either set both next and prev to this or next to
> this and prev to null.
>
>
OK, now only next is ever self-linked.

this.prev = null;
this.next = this;   // mark as finalized

Re: RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

[Adam Farley8]

On 14/02/2018 14:13, Adam Farley8 wrote:
>> Hi All,
>>
>> -- Short version --
>>
>> Could a committer please take the fix for JDK-8190187 (full code 
included
>> in the bug) and:
>>
>> 1) Complete the CSR process for the new JNI Return code.
>> 2) Commit the changes that contain (a) the new return code, and (b) the
>> non-Hotspot code that handles the new code.
> The patches attached to the JIRA issue are missing the changes to the 
> JVM TI spec (jvmti.xml). 

I'm not seeing the JNI return codes in that file. Are you after one of 
those
dated updates near the bottom?
e.g.
```
  
  Added JNI_SILENT_EXIT return code for early exits without errors.
  java.c's ParseArguments function now sets pret value to 2 for a NULL 
pwhat.
 This allows us to clearly identify when no class was set, but no 
other error has occurred.
 This is undone in java.c's ContinueInNewThread method, so the 
surface behaviour does not change. 
  
```

> There is also text to be written for the JNI spec if this proposal goes 
> ahead.

I assume you mean the "RETURNS" section of the JNI_CreateJavaVM 
bit on the invocation.html web page. Something like this?

```
RETURNS:
Returns JNI_OK on success; returns a suitable JNI error code (a negative 
number) on failure.

The sole exception is a silent exit, which returns JNI error code 
JNI_SILENT_EXIT. 
This indicates that the VM cannot be used, but that this is the intended 
behaviour for the 
arguments used. E.g. -Xlog:help (which prints help output and then 
destroys the VM)
```

>
> I don't agree that the launcher should be looking for 
> "-agentlib:jdwp=help" in the command line as it's just one of many ways 
> that the debugger agent might be started (e.g. -Xrunjdwp:, 
> _JAVA_TOOLS_OPTIONS, ...).

We can avoid that by finding a way around this line in ContinueInNewThread 
(java.c):

```
return (ret != 0) ? ret : rslt;
```

I have devised a means to do this, as outlined in the jvmti.xml change 
above. I put the
changes into a recent clone of jdk/jdk, and attached the hg diff, along 
with an improved
test.



If you have Author or Committer privileges, could you add the files to the 
bug please?

>
>> Backporting would be appreciated, but is optional.
> I don't think these changes are appropriate to backport as they include 
> specification changes/
>
>-Alan

This is fair.

- Adam

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: RFR: 8041626: [Event Request] Shutdown reason

[Roger Riggs]

Hi Robin,

Looks fine to me.

(How is this tested?, Normal, exceptional, etc.)

Thanks, Roger


On 2/15/2018 8:35 AM, Robin Westberg wrote:

Hi again,

Here’s the (hopefully) final version:

Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ 

Incremental: 
http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ 



Best regards,
Robin

On 14 Feb 2018, at 16:15, Robin Westberg > wrote:


Hi Roger,

On 13 Feb 2018, at 16:17, Roger Riggs > wrote:


Hi Robin,

It looks like the status argument to BeforeHalt is discarded in 
JVM_BeforeHalt

and is not inserted into the event.
That suggests it should be removed all the way back to 
Shutdown.beforeHalt.


You are right, my thinking was that the interface wouldn’t need to be 
changed if we decided to revisit the event in the future and add the 
status code. But that is perhaps unlikely, so can certainly remove 
the argument for now.


Best regards,
Robin



Roger



On 2/13/2018 9:59 AM, Robin Westberg wrote:

Hi Alan,

On 12 Feb 2018, at 09:02, Alan Bateman > wrote:




On 12/02/2018 07:07, David Holmes wrote:
Okay, I’ve removed the code related to the status field, 
certainly makes the change a bit less intrusive.


Updated webrev: 
http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/ 

Incremental: 
http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/ 


This looks much cleaner/neater to me - thanks.


The updates to Runtime/Shutdown seems okay.

Thanks for reviewing!

Best regards,
Robin


-Alan

Re: RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

[sangheon.kim]

Hi Thomas,

On 02/15/2018 03:32 AM, Thomas Schatzl wrote:

Hi,

On Wed, 2018-02-14 at 13:45 -0800, sangheon.kim wrote:

Hi all,

Could I have some reviews for CMM removal?
This is closed CR but some public codes also need small
modifications.
This CR is for removing stuff related to an Oracle JDK
module/package.
Changes are just removing CMM from lists or in a test to skip the
testing logic.

CR: https://bugs.openjdk.java.net/browse/JDK-8193909
Webrev: http://cr.openjdk.java.net/~sangheki/8193909/webrev.0
Testing: hs-tier1~5, jdk1~3, open/test/jdk:jdk_core


   looks good.

Thank you for your review!

Sangheon




Thomas

Re: 8193819: Error message when module does not have a ModuleMainClass attribute is confusing

[Sundararajan Athijegannathan]

+1

-Sundar

On 15/02/18, 7:30 PM, Alan Bateman wrote:
There's a typo in the launcher resource used for the error message 
when `java -m` fails because the module doesn't have a main class. The 
name of the attribute is "ModuleMainClass", not "MainClass". Trivial 
change.


-Alan


diff --git 
a/src/java.base/share/classes/sun/launcher/resources/launcher.properties 
b/src/java.base/share/classes/sun/launcher/resources/launcher.properties
--- 
a/src/java.base/share/classes/sun/launcher/resources/launcher.properties
+++ 
b/src/java.base/share/classes/sun/launcher/resources/launcher.properties

@@ -220,7 +220,7 @@
 Error: The JavaFX launchApplication method has the wrong 
signature, it\n\

 must be declared static and return a value of type void
 java.launcher.module.error1=\
-module {0} does not have a MainClass attribute, use -m 
/
+module {0} does not have a ModuleMainClass attribute, use -m 
/

 java.launcher.module.error2=\
 Error: Could not find or load main class {0} in module {1}
 java.launcher.module.error3=\

8193819: Error message when module does not have a ModuleMainClass attribute is confusing

[Alan Bateman]

There's a typo in the launcher resource used for the error message when 
`java -m` fails because the module doesn't have a main class. The name 
of the attribute is "ModuleMainClass", not "MainClass". Trivial change.


-Alan


diff --git 
a/src/java.base/share/classes/sun/launcher/resources/launcher.properties 
b/src/java.base/share/classes/sun/launcher/resources/launcher.properties

--- a/src/java.base/share/classes/sun/launcher/resources/launcher.properties
+++ b/src/java.base/share/classes/sun/launcher/resources/launcher.properties
@@ -220,7 +220,7 @@
 Error: The JavaFX launchApplication method has the wrong 
signature, it\n\

 must be declared static and return a value of type void
 java.launcher.module.error1=\
-    module {0} does not have a MainClass attribute, use -m 
/
+    module {0} does not have a ModuleMainClass attribute, use -m 
/

 java.launcher.module.error2=\
 Error: Could not find or load main class {0} in module {1}
 java.launcher.module.error3=\

Re: RFR: 8041626: [Event Request] Shutdown reason

[Robin Westberg]

Hi again,

Here’s the (hopefully) final version:

Full: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.02/ 

Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01-02/ 


Best regards,
Robin

> On 14 Feb 2018, at 16:15, Robin Westberg  wrote:
> 
> Hi Roger,
> 
>> On 13 Feb 2018, at 16:17, Roger Riggs  wrote:
>> 
>> Hi Robin,
>> 
>> It looks like the status argument to BeforeHalt is discarded in 
>> JVM_BeforeHalt
>> and is not inserted into the event.
>> That suggests it should be removed all the way back to Shutdown.beforeHalt.
> 
> You are right, my thinking was that the interface wouldn’t need to be changed 
> if we decided to revisit the event in the future and add the status code. But 
> that is perhaps unlikely, so can certainly remove the argument for now.
> 
> Best regards,
> Robin
> 
>> 
>> Roger
>> 
>> 
>> 
>> On 2/13/2018 9:59 AM, Robin Westberg wrote:
>>> Hi Alan,
>>> 
 On 12 Feb 2018, at 09:02, Alan Bateman  wrote:
 
 
 
 On 12/02/2018 07:07, David Holmes wrote:
>> Okay, I’ve removed the code related to the status field, certainly makes 
>> the change a bit less intrusive.
>> 
>> Updated webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.01/
>> Incremental: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00-01/
> This looks much cleaner/neater to me - thanks.
> 
 The updates to Runtime/Shutdown seems okay.
>>> Thanks for reviewing!
>>> 
>>> Best regards,
>>> Robin
>>> 
 -Alan
>> 
> 

Re: RFR: 8196959: NullPointerException in discovery003.java

[Jim Laskey]

+1


> On Feb 14, 2018, at 1:24 AM, Srinivas Dama  wrote:
> 
> Jim,
> 
> May I have second review for this thread in core-libs-dev group.
> 
> Regards,
> Srinivas
> 
> - Forwarded Message -
> From: sundararajan.athijegannat...@oracle.com
> To: srinivas.d...@oracle.com
> Cc: core-libs-dev@openjdk.java.net
> Sent: Monday, February 12, 2018 6:25:08 PM GMT +05:30 Chennai, Kolkata, 
> Mumbai, New Delhi
> Subject: Re: RFR: 8196959: NullPointerException in discovery003.java
> 
> Looks good.
> 
> -Sundar
> 
> On 12/02/18, 5:31 PM, Srinivas Dama wrote:
>> Hi,
>> 
>> Please review http://cr.openjdk.java.net/~sdama/8196959/webrev.00/
>> for https://bugs.openjdk.java.net/browse/JDK-8196959
>> 
>> This is small modification of fix for 
>> https://bugs.openjdk.java.net/browse/JDK-8011697.
>> Fix is to handle the corner case where the engineName is null.
>> 
>> 
>> Regards,
>> Srinivas

Re: 8193818 : Remove unused single_step field from java.lang.Thread

[David Holmes]

On 15/02/2018 9:42 PM, Alan Bateman wrote:

On 19/12/2017 11:06, Alan Bateman wrote:
I've been going through the fields in java.lang.Thread and I'm 
wondering if this field can be removed:


    /* Whether or not to single_step this thread. */
    private boolean single_step;

This field was used in the original Classic VM (pre-OpenJDK history). 
It doesn't appear to be used in the HotSpot VM.


Does anyone know of any reason to keep it? Are there other VMs using 
it by any chance?
No one screamed so I'd like to go ahead and remove this field. I've 
created JDK-8193818 to track it, the change (below) is trivial.


Looks good to me :)

Thanks,
David


-Alan


--- a/src/java.base/share/classes/java/lang/Thread.java
+++ b/src/java.base/share/classes/java/lang/Thread.java
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1994, 2018, 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
@@ -150,9 +150,6 @@
  private Thread threadQ;
  private long   eetop;

-    /* Whether or not to single_step this thread. */
-    private boolean single_step;
-
  /* Whether or not the thread is a daemon thread. */
  private boolean daemon = false;

Re: 8193818 : Remove unused single_step field from java.lang.Thread

[Lance Andersen]

+1
> On Feb 15, 2018, at 6:42 AM, Alan Bateman  wrote:
> 
> On 19/12/2017 11:06, Alan Bateman wrote:
>> I've been going through the fields in java.lang.Thread and I'm wondering if 
>> this field can be removed:
>> 
>> /* Whether or not to single_step this thread. */
>> private boolean single_step;
>> 
>> This field was used in the original Classic VM (pre-OpenJDK history). It 
>> doesn't appear to be used in the HotSpot VM.
>> 
>> Does anyone know of any reason to keep it? Are there other VMs using it by 
>> any chance?
> No one screamed so I'd like to go ahead and remove this field. I've created 
> JDK-8193818 to track it, the change (below) is trivial.
> 
> -Alan
> 
> 
> --- a/src/java.base/share/classes/java/lang/Thread.java
> +++ b/src/java.base/share/classes/java/lang/Thread.java
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1994, 2018, 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
> @@ -150,9 +150,6 @@
>  private Thread threadQ;
>  private long   eetop;
> 
> -/* Whether or not to single_step this thread. */
> -private boolean single_step;
> -
>  /* Whether or not the thread is a daemon thread. */
>  private boolean daemon = false;

 
  

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

8193818 : Remove unused single_step field from java.lang.Thread

[Alan Bateman]

On 19/12/2017 11:06, Alan Bateman wrote:
I've been going through the fields in java.lang.Thread and I'm 
wondering if this field can be removed:


    /* Whether or not to single_step this thread. */
    private boolean single_step;

This field was used in the original Classic VM (pre-OpenJDK history). 
It doesn't appear to be used in the HotSpot VM.


Does anyone know of any reason to keep it? Are there other VMs using 
it by any chance?
No one screamed so I'd like to go ahead and remove this field. I've 
created JDK-8193818 to track it, the change (below) is trivial.


-Alan


--- a/src/java.base/share/classes/java/lang/Thread.java
+++ b/src/java.base/share/classes/java/lang/Thread.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1994, 2018, 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
@@ -150,9 +150,6 @@
 private Thread threadQ;
 private long   eetop;

-    /* Whether or not to single_step this thread. */
-    private boolean single_step;
-
 /* Whether or not the thread is a daemon thread. */
 private boolean daemon = false;

Re: RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

[Thomas Schatzl]

Hi,

On Wed, 2018-02-14 at 13:45 -0800, sangheon.kim wrote:
> Hi all,
> 
> Could I have some reviews for CMM removal?
> This is closed CR but some public codes also need small
> modifications. 
> This CR is for removing stuff related to an Oracle JDK
> module/package.
> Changes are just removing CMM from lists or in a test to skip the 
> testing logic.
> 
> CR: https://bugs.openjdk.java.net/browse/JDK-8193909
> Webrev: http://cr.openjdk.java.net/~sangheki/8193909/webrev.0
> Testing: hs-tier1~5, jdk1~3, open/test/jdk:jdk_core
> 

  looks good.

Thomas

Re: RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

[Adam Farley8]

On 15/02/2018 12:13 AM, Adam Farley8 wrote:
>> Hi All,
>> 
>> -- Short version --
>> 
>> Could a committer please take the fix for JDK-8190187 (full code 
included
>> in the bug) and:
>> 
>> 1) Complete the CSR process for the new JNI Return code.
>> 2) Commit the changes that contain (a) the new return code, and (b) the
>> non-Hotspot code that handles the new code.
>
> As I stated here:
>
> 
https://urldefense.proofpoint.com/v2/url?u=http-3A__mail.openjdk.java.net_pipermail_core-2Dlibs-2Ddev_2017-2DOctober_049597.html=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=P5m8KWUXJf-CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=OUKJytaKUZmaM4zCoF2GSmIrYLPEE4cSP1LkBgAIduo=DIuL0lNnE0xXAqlADCHi8uv2u6kcKcOWbob0WHHArSU=
>
> "I can file a bug for this, but it will take some work to see how to fit
> this into the existing specifications and file CSR requests for those
> changes. This won't make 18.3 (or whatever it gets called). You will
> need a "champion" to help flesh this out in full and work with you on
> those CSR requests. I can't volunteer to do that at this time."
>
> Nobody has offered to champion this. Sorry.
>
> David
> -

Yup, I remember you saying that David.

I plan to work on the issues you raised in your previous email, and 
continue to post messages in these lists until someone who has
free cycles spots my emails and agrees to commit the code.

It'll happen sooner or later. :)

- Adam

>
>> Backporting would be appreciated, but is optional.
>> 
>> Also optional: Commit the jdwp code that serves as an example for how 
this
>> code should be used. CSR again, unfortunately.
>> 
>> 
>> -- Long Version --
>> 
>> We have a bug in OpenJDK where if you pass an info-only option (like
>> -agentlib:jdwp=help) in through the JNI interface, it can exit your 
code
>> with RC 0.
>> 
>> I think this is a bug because if you planned to do anything after 
starting
>> a Java VM, you have to do it in an exit hook.
>> 
>> If an info-only option is used, exit(0) should not happen. Instead, the
>> user's code should be told the VM cannot be used.
>> 
>> Bug Link: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8190187=DwICaQ=jf_iaSHvJObTbx-siA1ZOg=P5m8KWUXJf-CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=OUKJytaKUZmaM4zCoF2GSmIrYLPEE4cSP1LkBgAIduo=P_S35LF3YxaxKQWY7-svCzm_WvvcUU0nSx8QP9dH9O0=
>> 
>> I have proposed the creation of a new JNI return code indicating a 
"silent
>> exit", where the vm encountered no error, but that it is not in a fit
>> state to be used.
>> 
>> See the link for an implementation of this, along with a handy test.
>> 
>> In short, if you agree that this is a problem, we need a champion to:
>> 
>> 1) Complete the CSR process for the new JNI Return code.
>> 2) Commit the changes that contain (a) the new return code, and (b) the
>> non-Hotspot code that handles the new code.
>> 
>> Optionally, if you want to commit the code containing an example of the
>> fix, you will need to:
>> 
>> 3) Commit the Hotspot code that channels the new return code.
>> 4) Complete the CSR process for the jdwp behaviour change.
>> 5) Commit the jdwp code.
>> 
>> Note that all of this code has *already been written*. This should be 
an
>> easy commit once the CSR is done with.
>> 
>> Best Regards
>> 
>> Adam Farley
>> 
>> 
>> 
>> Best Regards
>> 
>> Adam Farley
>> 
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with 
number
>> 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
>> 
>
>

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[Alan Bateman]

On 14/02/2018 23:52, yumin qi wrote:

Brian,

  Updated using RuntimeException, which will not need -ea so removed.
http://cr.openjdk.java.net/~minqi/8194154/webrev1/ 



Can you explain why resolve has been changed to call normalize 
parent+child? I can't help thinking there is something else going on if 
this change is needed.


I see the mails about the test so I'll skip that except to say that it 
should not need "require". The test should be able to run on all platforms.


-Alan

Re: RFR: JDK-8197988, mach5 T2 test javax/net/ssl/interop/ClientHelloChromeInterOp.java failed after JDK-8164278

[Xueming Shen]

On 2/15/18, 12:15 AM, Alan Bateman wrote:

On 15/02/2018 07:49, Xueming Shen wrote:

Hi,

Please help review the change for  JDK-819798

issue: https://bugs.openjdk.java.net/browse/JDK-8197988
webrev: http://cr.openjdk.java.net/~sherman/8197988/webrev

The change for JDK-8164278 caused a decoding regression. A Tier2
test case,  javax/net/ssl/interop/ClientHelloChromeInterOp.java, that
uses mime-base64-decoder failed.

The "condition" that triggers the decoding to go the fast path is 
incorrect in

the code pushed for JDK-819798.

The intent here is to only start/try the fast path when we are at the 
beginning
of a 4-byte legal base64 unit. The correct/appropriate check is to 
see if
shiftto == 18, which means the decoder is still at the beginning of 
4-byte
unit. ( bits == 0 is a false indicator, as bits might include decoded 
byte value of

0). The fix is a relatively easy/one line change.
This looks okay. Should we create another issue to add more tests in 
this area to avoid the SSL code finding issues in this area again.


Yes, I will do that tomorrow. Just need to get the mach5 going first 
tonight.


thanks,
Sherman

Re: RFR: JDK-8197988, mach5 T2 test javax/net/ssl/interop/ClientHelloChromeInterOp.java failed after JDK-8164278

[Alan Bateman]

On 15/02/2018 07:49, Xueming Shen wrote:

Hi,

Please help review the change for  JDK-819798

issue: https://bugs.openjdk.java.net/browse/JDK-8197988
webrev: http://cr.openjdk.java.net/~sherman/8197988/webrev

The change for JDK-8164278 caused a decoding regression. A Tier2
test case,  javax/net/ssl/interop/ClientHelloChromeInterOp.java, that
uses mime-base64-decoder failed.

The "condition" that triggers the decoding to go the fast path is 
incorrect in

the code pushed for JDK-819798.

The intent here is to only start/try the fast path when we are at the 
beginning

of a 4-byte legal base64 unit. The correct/appropriate check is to see if
shiftto == 18, which means the decoder is still at the beginning of 
4-byte
unit. ( bits == 0 is a false indicator, as bits might include decoded 
byte value of

0). The fix is a relatively easy/one line change.
This looks okay. Should we create another issue to add more tests in 
this area to avoid the SSL code finding issues in this area again.


-Alan