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

[Peter Levart]

Hi Martin,

On 02/14/2018 07:58 AM, Peter Levart wrote:
It may be that the intent was to refrain from using the shared 'lock' 
lock for the 2nd and subsequent calls to runFinalizer() and only use 
the more fine-grained 'this' lock in this case?


If someone was able to call runFinalizer() on the same instance in a 
loop he could prevent or slow-down normal processing of other 
Finalizer(s) if the shared 'lock' was always employed. What do you think? 


I checked all uses of runFinalizer() and I don't think it can be called 
more than twice for the same Finalizer instance (for example if there is 
a race between runAllFinalizers() and processing of Finalizers taken 
from the ReferenceQueue). So your patch is a nice simplification.


Regards, Peter

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

[Peter Levart]

Hi Again,

While studying the code of Finalizer I found a strange discrepancy 
between handling of 'unfinalized' field during processing of 
Finalizer(s) taken from ReferenceQueue and taken from the 'unfinalized' 
pointer itself (as in runAllFinalizers()). The remove() method is as 
follows:


    private void remove() {
    synchronized (lock) {
    if (unfinalized == this) {
    if (this.next != null) {
    unfinalized = this.next;
    } else {
    unfinalized = this.prev;
    }
    }
    if (this.next != null) {
    this.next.prev = this.prev;
    }
    if (this.prev != null) {
    this.prev.next = this.next;
    }
    this.next = this;   /* Indicates that this has been 
finalized */

    this.prev = this;
    }
    }

When Finalizer(s) are taken from ReferenceQueue, they are processed in 
arbitrary order, so once in a while it can happen that the Finalizer at 
the head of the list (pointed to by 'unfinalized') is processed this 
way. The body of the 1st if statement in above method is executed in 
such situation:


    if (unfinalized == this) {
    if (this.next != null) {
    unfinalized = this.next;
    } else {
    unfinalized = this.prev;
    }
    }

But I can't figure out a situation when this.next would be null while 
this.prev would be non-null. In other words, when 'unfinalized' would 
not point to the head of the list. For comparison, when Finalizer(s) are 
processed from the 'unfinalized' pointer directly (as in 
runAllFinalizers()), the following is executed:


                for (;;) {
    Finalizer f;
    synchronized (lock) {
    f = unfinalized;
    if (f == null) break;
    unfinalized = f.next;
    }
    f.runFinalizer(jla);
    }

... so it is assumed that the 'unfinalized' always points to the head. 
If this was not true, not all Finalizer(s) would be processed.


So what I'm asking is whether this simplified if statement in remove() 
would be equivalent:


    if (unfinalized == this) {
    unfinalized = this.next;
    }

Or am I missing some situation?

Regards, Peter

On 02/14/2018 09:19 AM, Peter Levart wrote:

Hi Martin,

On 02/14/2018 07:58 AM, Peter Levart wrote:
It may be that the intent was to refrain from using the shared 'lock' 
lock for the 2nd and subsequent calls to runFinalizer() and only use 
the more fine-grained 'this' lock in this case?


If someone was able to call runFinalizer() on the same instance in a 
loop he could prevent or slow-down normal processing of other 
Finalizer(s) if the shared 'lock' was always employed. What do you 
think? 


I checked all uses of runFinalizer() and I don't think it can be 
called more than twice for the same Finalizer instance (for example if 
there is a race between runAllFinalizers() and processing of 
Finalizers taken from the ReferenceQueue). So your patch is a nice 
simplification.


Regards, Peter

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

[Peter Levart]

Hi,

I may have found a situation described here...

On 02/14/2018 09:47 AM, Peter Levart wrote:


When Finalizer(s) are taken from ReferenceQueue, they are processed in 
arbitrary order, so once in a while it can happen that the Finalizer 
at the head of the list (pointed to by 'unfinalized') is processed 
this way. The body of the 1st if statement in above method is executed 
in such situation:


    if (unfinalized == this) {
    if (this.next != null) {
    unfinalized = this.next;
    } else {
    unfinalized = this.prev;
    }
    }

But I can't figure out a situation when this.next would be null while 
this.prev would be non-null. In other words, when 'unfinalized' would 
not point to the head of the list. 


This is the situation:

unfinalized
    |
    v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

runAllFinalizers() is executed and the 1st pass of the for loop takes 
the Finalizer[1] from the 'unfinalized' pointer and moves the pointer to 
Finalizer[2]:


unfinalized 
   |
   v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

...now finalizer thread kicks-in and takes the Finalizer[2] from the 
ReferenceQueue and calls remove(). The 1st if statement in remove() 
would evaluate the condition to true, as 'unfinalized' now points to 
Finalizer[2], so the body of 1st if would make unfinalized point back to 
Finalizer[1]:


unfinalized
    |
    v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

...and the remaining remove() code would unlink the Finalizer[2] from 
the list, resulting in this state of the list:


unfinalized
    |
    v
Finalizer[1].next --> null
Finalizer[1].prev --> null

This might be seen as a "correct" state, because Finalizer[1] has not 
been removed from the list yet although it is in the process of being 
removed because the next thing runAllFinalizers() loop would do is it 
would call runFinalizer() on it. So the simplified handling in remove():


    if (unfinalized == this) {
        unfinalized = this.next;
    }

...would result in correct behavior too. Even more so because if the 2nd 
call to runAllFinalizers() was being executed concurrently (I don't know 
if this is possible though), the Finalizer[1] from above situation would 
be taken by the concurrent call again and its runFinalizer() executed. 
runFinalizer() is idempotent so no harm done here, but...


Regards, Peter

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

[Peter Levart]

I could even claim that simplifying the if statement in remove() to:

    if (unfinalized == this) {
        unfinalized = this.next;
    }

makes checking for hasBeenFinalized() in runFinalizer() redundant as it 
would not be possible for runFinalizer() to be called more than once for 
each Finalizer instance because:
- ReferenceQueue never returns the same Reference instance twice or more 
times.
- 'unfinalized' will never point back to the same Finalizer instance for 
the 2nd time, because it always "travels" in the forward direction 
(unfinalized = unfinalized.next).


Regards, Peter

(I rest now).

On 02/14/2018 10:24 AM, Peter Levart wrote:

Hi,

I may have found a situation described here...

On 02/14/2018 09:47 AM, Peter Levart wrote:


When Finalizer(s) are taken from ReferenceQueue, they are processed 
in arbitrary order, so once in a while it can happen that the 
Finalizer at the head of the list (pointed to by 'unfinalized') is 
processed this way. The body of the 1st if statement in above method 
is executed in such situation:


    if (unfinalized == this) {
    if (this.next != null) {
    unfinalized = this.next;
    } else {
    unfinalized = this.prev;
    }
    }

But I can't figure out a situation when this.next would be null while 
this.prev would be non-null. In other words, when 'unfinalized' would 
not point to the head of the list. 


This is the situation:

unfinalized
    |
    v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

runAllFinalizers() is executed and the 1st pass of the for loop takes 
the Finalizer[1] from the 'unfinalized' pointer and moves the pointer 
to Finalizer[2]:


unfinalized 
   |
   v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

...now finalizer thread kicks-in and takes the Finalizer[2] from the 
ReferenceQueue and calls remove(). The 1st if statement in remove() 
would evaluate the condition to true, as 'unfinalized' now points to 
Finalizer[2], so the body of 1st if would make unfinalized point back 
to Finalizer[1]:


unfinalized
    |
    v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

...and the remaining remove() code would unlink the Finalizer[2] from 
the list, resulting in this state of the list:


unfinalized
    |
    v
Finalizer[1].next --> null
Finalizer[1].prev --> null

This might be seen as a "correct" state, because Finalizer[1] has not 
been removed from the list yet although it is in the process of being 
removed because the next thing runAllFinalizers() loop would do is it 
would call runFinalizer() on it. So the simplified handling in remove():


    if (unfinalized == this) {
        unfinalized = this.next;
    }

...would result in correct behavior too. Even more so because if the 
2nd call to runAllFinalizers() was being executed concurrently (I 
don't know if this is possible though), the Finalizer[1] from above 
situation would be taken by the concurrent call again and its 
runFinalizer() executed. runFinalizer() is idempotent so no harm done 
here, but...


Regards, Peter

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

[Peter Levart]

I take back this claim. Of course the the following race is possible:

- Thread1: calls runAllFinalizers and takes a Finalizer from 
'unprocessed' list.
- Thread2: takee the same Finalizer instance from ReferenceQueue and 
calls runFinalizer()

- Thread1: calls runFinalizer() with the same instance for the 2nd time now.

Regards, Peter

On 02/14/2018 10:39 AM, Peter Levart wrote:

I could even claim that simplifying the if statement in remove() to:

    if (unfinalized == this) {
        unfinalized = this.next;
    }

makes checking for hasBeenFinalized() in runFinalizer() redundant as 
it would not be possible for runFinalizer() to be called more than 
once for each Finalizer instance because:
- ReferenceQueue never returns the same Reference instance twice or 
more times.
- 'unfinalized' will never point back to the same Finalizer instance 
for the 2nd time, because it always "travels" in the forward direction 
(unfinalized = unfinalized.next).


Regards, Peter

(I rest now).

On 02/14/2018 10:24 AM, Peter Levart wrote:

Hi,

I may have found a situation described here...

On 02/14/2018 09:47 AM, Peter Levart wrote:


When Finalizer(s) are taken from ReferenceQueue, they are processed 
in arbitrary order, so once in a while it can happen that the 
Finalizer at the head of the list (pointed to by 'unfinalized') is 
processed this way. The body of the 1st if statement in above method 
is executed in such situation:


    if (unfinalized == this) {
    if (this.next != null) {
    unfinalized = this.next;
    } else {
    unfinalized = this.prev;
    }
    }

But I can't figure out a situation when this.next would be null 
while this.prev would be non-null. In other words, when 
'unfinalized' would not point to the head of the list. 


This is the situation:

unfinalized
    |
    v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

runAllFinalizers() is executed and the 1st pass of the for loop takes 
the Finalizer[1] from the 'unfinalized' pointer and moves the pointer 
to Finalizer[2]:


unfinalized 
   |
   v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

...now finalizer thread kicks-in and takes the Finalizer[2] from the 
ReferenceQueue and calls remove(). The 1st if statement in remove() 
would evaluate the condition to true, as 'unfinalized' now points to 
Finalizer[2], so the body of 1st if would make unfinalized point back 
to Finalizer[1]:


unfinalized
    |
    v
Finalizer[1].next --> Finalizer[2].next --> null
Finalizer[2].prev --> Finalizer[1].prev --> null

...and the remaining remove() code would unlink the Finalizer[2] from 
the list, resulting in this state of the list:


unfinalized
    |
    v
Finalizer[1].next --> null
Finalizer[1].prev --> null

This might be seen as a "correct" state, because Finalizer[1] has not 
been removed from the list yet although it is in the process of being 
removed because the next thing runAllFinalizers() loop would do is it 
would call runFinalizer() on it. So the simplified handling in remove():


    if (unfinalized == this) {
        unfinalized = this.next;
    }

...would result in correct behavior too. Even more so because if the 
2nd call to runAllFinalizers() was being executed concurrently (I 
don't know if this is possible though), the Finalizer[1] from above 
situation would be taken by the concurrent call again and its 
runFinalizer() executed. runFinalizer() is idempotent so no harm done 
here, but...


Regards, Peter

[1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

[Volker Simonis]

Hi,

can I please get a review for the following tiny fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
https://bugs.openjdk.java.net/browse/JDK-8197927

The new Java 10 specification makes the 'java.vendor.version' property
mandatory [1] but the current implementations doesn't allow to set it
to an empty string.

Currently, if we don't configure the build with
--with-vendor-version=XXX the 'java.vendor.version' property won't be
set at all, which violates the spec. Setting it to an empty string
will be ignored and has the same behavior like not setting it at all.
This also makes default OpenJDK builds (which haven't been configured
with --with-vendor-version non-compliant).

The fix is trivial: unconditionally set the 'java.vendor.version'
property to the value of VENDOR_VERSION_STRING in
VersionProps.java.template, even if VENDOR_VERSION_STRING is the empty
string (which is the default if --with-vendor-version wasn't given at
config time).

Thank you and best regards,
Volker


[1] 
http://cr.openjdk.java.net/~iris/se/10/pfd/java-se-10-pfd-spec/apidiffs/java/lang/System-report.html#method:getProperties()

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

[Peter Levart]

Hi Martin,

On 02/14/2018 10:58 AM, Peter Levart wrote:


I take back this claim. Of course the the following race is possible:

- Thread1: calls runAllFinalizers and takes a Finalizer from 
'unprocessed' list.
- Thread2: takee the same Finalizer instance from ReferenceQueue and 
calls runFinalizer()
- Thread1: calls runFinalizer() with the same instance for the 2nd 
time now.


... but this could be "fixed" if the taking of next Finalizer from 
'unfinalized' list and removing it from the same list was a single 
atomic operation. What do you say of the following further simplification:


http://cr.openjdk.java.net/~plevart/jdk-dev/8197812_Data_race_in_Finalizer/webrev.01/

Regards, Peter



Regards, Peter

On 02/14/2018 10:39 AM, Peter Levart wrote:

I could even claim that simplifying the if statement in remove() to:

    if (unfinalized == this) {
        unfinalized = this.next;
    }

makes checking for hasBeenFinalized() in runFinalizer() redundant as 
it would not be possible for runFinalizer() to be called more than 
once for each Finalizer instance because:
- ReferenceQueue never returns the same Reference instance twice or 
more times.
- 'unfinalized' will never point back to the same Finalizer instance 
for the 2nd time, because it always "travels" in the forward 
direction (unfinalized = unfinalized.next).


Regards, Peter

(I rest now).

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

[Thomas Stüfe]

Fix is fine, trivial and I do not think there is any risk attached to it. I
am not in any position to comment whether this is P1. Copyright year needs
adjusting.

Kind Regards, Thomas

On Wed, Feb 14, 2018 at 11:20 AM, Volker Simonis 
wrote:

> Hi,
>
> can I please get a review for the following tiny fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
> https://bugs.openjdk.java.net/browse/JDK-8197927
>
> The new Java 10 specification makes the 'java.vendor.version' property
> mandatory [1] but the current implementations doesn't allow to set it
> to an empty string.
>
> Currently, if we don't configure the build with
> --with-vendor-version=XXX the 'java.vendor.version' property won't be
> set at all, which violates the spec. Setting it to an empty string
> will be ignored and has the same behavior like not setting it at all.
> This also makes default OpenJDK builds (which haven't been configured
> with --with-vendor-version non-compliant).
>
> The fix is trivial: unconditionally set the 'java.vendor.version'
> property to the value of VENDOR_VERSION_STRING in
> VersionProps.java.template, even if VENDOR_VERSION_STRING is the empty
> string (which is the default if --with-vendor-version wasn't given at
> config time).
>
> Thank you and best regards,
> Volker
>
>
> [1] http://cr.openjdk.java.net/~iris/se/10/pfd/java-se-10-pfd-
> spec/apidiffs/java/lang/System-report.html#method:getProperties()
>

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

[Peter Levart]

On 02/14/2018 11:49 AM, Peter Levart wrote:

Hi Martin,

On 02/14/2018 10:58 AM, Peter Levart wrote:


I take back this claim. Of course the the following race is possible:

- Thread1: calls runAllFinalizers and takes a Finalizer from 
'unprocessed' list.
- Thread2: takee the same Finalizer instance from ReferenceQueue and 
calls runFinalizer()
- Thread1: calls runFinalizer() with the same instance for the 2nd 
time now.


... but this could be "fixed" if the taking of next Finalizer from 
'unfinalized' list and removing it from the same list was a single 
atomic operation. What do you say of the following further 
simplification:


http://cr.openjdk.java.net/~plevart/jdk-dev/8197812_Data_race_in_Finalizer/webrev.01/ 



Even that has a flaw. Running the next unfinalized Finalizer from 
runAllFinalizers() does not prevent the same Finalizer to be returned 
from the ReferenceQueue. So the check must remain in place.


Sorry for this long monologue. I truly rest now.

Regards, Peter

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

[Volker Simonis]

Thanks Thomas.

The sole reason for making it P1 is that it is currently not possible
to build a Java 10 conforming version of OpenJDK with an empty vendor
version.

The specification doesn't mandate a non-empty vendor version and I
don't think OpenJDK should mandate on either.

Regards,
Volker

On Wed, Feb 14, 2018 at 11:50 AM, Thomas Stüfe  wrote:
> Fix is fine, trivial and I do not think there is any risk attached to it. I
> am not in any position to comment whether this is P1. Copyright year needs
> adjusting.
>
> Kind Regards, Thomas
>
> On Wed, Feb 14, 2018 at 11:20 AM, Volker Simonis 
> wrote:
>>
>> Hi,
>>
>> can I please get a review for the following tiny fix:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
>> https://bugs.openjdk.java.net/browse/JDK-8197927
>>
>> The new Java 10 specification makes the 'java.vendor.version' property
>> mandatory [1] but the current implementations doesn't allow to set it
>> to an empty string.
>>
>> Currently, if we don't configure the build with
>> --with-vendor-version=XXX the 'java.vendor.version' property won't be
>> set at all, which violates the spec. Setting it to an empty string
>> will be ignored and has the same behavior like not setting it at all.
>> This also makes default OpenJDK builds (which haven't been configured
>> with --with-vendor-version non-compliant).
>>
>> The fix is trivial: unconditionally set the 'java.vendor.version'
>> property to the value of VENDOR_VERSION_STRING in
>> VersionProps.java.template, even if VENDOR_VERSION_STRING is the empty
>> string (which is the default if --with-vendor-version wasn't given at
>> config time).
>>
>> Thank you and best regards,
>> Volker
>>
>>
>> [1]
>> http://cr.openjdk.java.net/~iris/se/10/pfd/java-se-10-pfd-spec/apidiffs/java/lang/System-report.html#method:getProperties()
>
>

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

[Alan Bateman]

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

Hi,

  I have update the webrev:
http://cr.openjdk.java.net/~minqi/8194154/webrev1/ 



  In this version, as suggested by Alan(thanks!), property of 
"user.dir" is cached and behave like it is 'read only'. The change 
made to *inux as well as windows. Since property of "user.dir" is 
cached, any changes via property setting for it has no effect.


This looks much better but you've removed a permission check from the 
Windows getUserPath implementation. That code the same permission check 
as the resolve method in the Unix implementation.


I think the test needs works. The simplest would be to call 
getCanonicalFile before changing the system property, then call it after 
and check that you an equal result. Also no need to hack the Properties 
object, you can use System.setProperty instead.


-Alan

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[David Holmes]

Adding in core-libs-dev as there's nothing related to hotspot directly here.

David

On 14/02/2018 9:32 PM, Adam Farley8 wrote:

Hi All,

Currently, diagnostic core files generated from OpenJDK seem to lump all
of the
native memory usages together, making it near-impossible for someone to
figure
out *what* is using all that memory in the event of a memory leak.

The OpenJ9 VM has a feature which allows it to track the allocation of
native
memory for Direct Byte Buffers (DBBs), and to supply that information into
the
cores when they are generated. This makes it a *lot* easier to find out
what is using
all that native memory, making memory leak resolution less like some dark
art, and
more like logical debugging.

To use this feature, there is a native method referenced in Unsafe.java.
To open
up this feature so that any VM can make use of it, the java code below
sets the
stage for it. This change starts letting people call DBB-specific methods
when
allocating native memory, and getting into the habit of using it.

Thoughts?

Best Regards

Adam Farley

P.S. Code:

diff --git
a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
--- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
+++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
@@ -85,7 +85,7 @@
  // Paranoia
  return;
  }
-UNSAFE.freeMemory(address);
+UNSAFE.freeDBBMemory(address);
  address = 0;
  Bits.unreserveMemory(size, capacity);
  }
@@ -118,7 +118,7 @@
  
  long base = 0;

  try {
-base = UNSAFE.allocateMemory(size);
+base = UNSAFE.allocateDBBMemory(size);
  } catch (OutOfMemoryError x) {
  Bits.unreserveMemory(size, cap);
  throw x;
diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
--- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
+++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
@@ -632,6 +632,26 @@
  }
  
  /**

+ * Allocates a new block of native memory for DirectByteBuffers, of
the
+ * given size in bytes.  The contents of the memory are
uninitialized;
+ * they will generally be garbage.  The resulting native pointer will
+ * never be zero, and will be aligned for all value types.  Dispose
of
+ * this memory by calling {@link #freeDBBMemory} or resize it with
+ * {@link #reallocateDBBMemory}.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #getByte(long)
+ * @see #putByte(long, byte)
+ */
+public long allocateDBBMemory(long bytes) {
+return allocateMemory(bytes);
+}
+
+/**
   * Resizes a new block of native memory, to the given size in bytes.
The
   * contents of the new block past the size of the old block are
   * uninitialized; they will generally be garbage.  The resulting
native
@@ -687,6 +707,27 @@
  }
  
  /**

+ * Resizes a new block of native memory for DirectByteBuffers, to the
+ * given size in bytes.  The contents of the new block past the size
of
+ * the old block are uninitialized; they will generally be garbage.
The
+ * resulting native pointer will be zero if and only if the requested
size
+ * is zero.  The resulting native pointer will be aligned for all
value
+ * types.  Dispose of this memory by calling {@link #freeDBBMemory},
or
+ * resize it with {@link #reallocateDBBMemory}.  The address passed
to
+ * this method may be null, in which case an allocation will be
performed.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #allocateDBBMemory
+ */
+public long reallocateDBBMemory(long address, long bytes) {
+return reallocateMemory(address, bytes);
+}
+
+/**
   * Sets all bytes in a given block of memory to a fixed value
   * (usually zero).
   *
@@ -918,6 +959,17 @@
  checkPointer(null, address);
  }
  
+/**

+ * Disposes of a block of native memory, as obtained from {@link
+ * #allocateDBBMemory} or {@link #reallocateDBBMemory}.  The address
passed
+ * to this method may be null, in which case no action is taken.
+ *
+ * @see #allocateDBBMemory
+ */
+public void freeDBBMemory(long address) {
+freeMemory(address);
+}
+
  /// random queries
  
  /**


Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and W

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[David Holmes]

On 14/02/2018 10:43 PM, David Holmes wrote:
Adding in core-libs-dev as there's nothing related to hotspot directly 
here.


Correction, this is of course leading to a proposed change in hotspot to 
implement the new Unsafe methods and perform the native memory tracking. 
Of course we already have NMT so the obvious question is how this will 
fit in with NMT?


David


David

On 14/02/2018 9:32 PM, Adam Farley8 wrote:

Hi All,

Currently, diagnostic core files generated from OpenJDK seem to lump all
of the
native memory usages together, making it near-impossible for someone to
figure
out *what* is using all that memory in the event of a memory leak.

The OpenJ9 VM has a feature which allows it to track the allocation of
native
memory for Direct Byte Buffers (DBBs), and to supply that information 
into

the
cores when they are generated. This makes it a *lot* easier to find out
what is using
all that native memory, making memory leak resolution less like some dark
art, and
more like logical debugging.

To use this feature, there is a native method referenced in Unsafe.java.
To open
up this feature so that any VM can make use of it, the java code below
sets the
stage for it. This change starts letting people call DBB-specific methods
when
allocating native memory, and getting into the habit of using it.

Thoughts?

Best Regards

Adam Farley

P.S. Code:

diff --git
a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
--- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
+++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
@@ -85,7 +85,7 @@
  // Paranoia
  return;
  }
-    UNSAFE.freeMemory(address);
+    UNSAFE.freeDBBMemory(address);
  address = 0;
  Bits.unreserveMemory(size, capacity);
  }
@@ -118,7 +118,7 @@
  long base = 0;
  try {
-    base = UNSAFE.allocateMemory(size);
+    base = UNSAFE.allocateDBBMemory(size);
  } catch (OutOfMemoryError x) {
  Bits.unreserveMemory(size, cap);
  throw x;
diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
--- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
+++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
@@ -632,6 +632,26 @@
  }
  /**
+ * Allocates a new block of native memory for DirectByteBuffers, of
the
+ * given size in bytes.  The contents of the memory are
uninitialized;
+ * they will generally be garbage.  The resulting native pointer 
will

+ * never be zero, and will be aligned for all value types.  Dispose
of
+ * this memory by calling {@link #freeDBBMemory} or resize it with
+ * {@link #reallocateDBBMemory}.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #getByte(long)
+ * @see #putByte(long, byte)
+ */
+    public long allocateDBBMemory(long bytes) {
+    return allocateMemory(bytes);
+    }
+
+    /**
   * Resizes a new block of native memory, to the given size in 
bytes.

The
   * contents of the new block past the size of the old block are
   * uninitialized; they will generally be garbage.  The resulting
native
@@ -687,6 +707,27 @@
  }
  /**
+ * Resizes a new block of native memory for DirectByteBuffers, to 
the

+ * given size in bytes.  The contents of the new block past the size
of
+ * the old block are uninitialized; they will generally be garbage.
The
+ * resulting native pointer will be zero if and only if the 
requested

size
+ * is zero.  The resulting native pointer will be aligned for all
value
+ * types.  Dispose of this memory by calling {@link #freeDBBMemory},
or
+ * resize it with {@link #reallocateDBBMemory}.  The address passed
to
+ * this method may be null, in which case an allocation will be
performed.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #allocateDBBMemory
+ */
+    public long reallocateDBBMemory(long address, long bytes) {
+    return reallocateMemory(address, bytes);
+    }
+
+    /**
   * Sets all bytes in a given block of memory to a fixed value
   * (usually zero).
   *
@@ -918,6 +959,17 @@
  checkPointer(null, address);
  }
+    /**
+ * Disposes of a block of native memory, as obtained from {@link
+ * #allocateDBBMemory} or {@link #reallocateDBBMemory}.  The address
passed
+ * to this method may be null, in which 

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[Adam Farley8]

> Adding in core-libs-dev as there's nothing related to hotspot directly 
here.
>
> David

Thought it was best to pass this through hotspot lists first, as full 
completion
of the native side of things will probably require hotspot changes.

You're quite right though, I should have cc'd hotspot *and* core libs.

- Adam

>
> On 14/02/2018 9:32 PM, Adam Farley8 wrote:
>> Hi All,
>> 
>> Currently, diagnostic core files generated from OpenJDK seem to lump 
all
>> of the
>> native memory usages together, making it near-impossible for someone to
>> figure
>> out *what* is using all that memory in the event of a memory leak.
>> 
>> The OpenJ9 VM has a feature which allows it to track the allocation of
>> native
>> memory for Direct Byte Buffers (DBBs), and to supply that information 
into
>> the
>> cores when they are generated. This makes it a *lot* easier to find out
>> what is using
>> all that native memory, making memory leak resolution less like some 
dark
>> art, and
>> more like logical debugging.
>> 
>> To use this feature, there is a native method referenced in 
Unsafe.java.
>> To open
>> up this feature so that any VM can make use of it, the java code below
>> sets the
>> stage for it. This change starts letting people call DBB-specific 
methods
>> when
>> allocating native memory, and getting into the habit of using it.
>> 
>> Thoughts?
>> 
>> Best Regards
>> 
>> Adam Farley
>> 
>> P.S. Code:
>> 
>> diff --git
>> a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>> b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>> --- 
a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>> +++ 
b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>> @@ -85,7 +85,7 @@
>>   // Paranoia
>>   return;
>>   }
>> -UNSAFE.freeMemory(address);
>> +UNSAFE.freeDBBMemory(address);
>>   address = 0;
>>   Bits.unreserveMemory(size, capacity);
>>   }
>> @@ -118,7 +118,7 @@
>> 
>>   long base = 0;
>>   try {
>> -base = UNSAFE.allocateMemory(size);
>> +base = UNSAFE.allocateDBBMemory(size);
>>   } catch (OutOfMemoryError x) {
>>   Bits.unreserveMemory(size, cap);
>>   throw x;
>> diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>> b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>> --- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>> +++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>> @@ -632,6 +632,26 @@
>>   }
>> 
>>   /**
>> + * Allocates a new block of native memory for DirectByteBuffers, 
of
>> the
>> + * given size in bytes.  The contents of the memory are
>> uninitialized;
>> + * they will generally be garbage.  The resulting native pointer 
will
>> + * never be zero, and will be aligned for all value types. Dispose
>> of
>> + * this memory by calling {@link #freeDBBMemory} or resize it with
>> + * {@link #reallocateDBBMemory}.
>> + *
>> + * @throws RuntimeException if the size is negative or too large
>> + *  for the native size_t type
>> + *
>> + * @throws OutOfMemoryError if the allocation is refused by the
>> system
>> + *
>> + * @see #getByte(long)
>> + * @see #putByte(long, byte)
>> + */
>> +public long allocateDBBMemory(long bytes) {
>> +return allocateMemory(bytes);
>> +}
>> +
>> +/**
>>* Resizes a new block of native memory, to the given size in 
bytes.
>> The
>>* contents of the new block past the size of the old block are
>>* uninitialized; they will generally be garbage.  The resulting
>> native
>> @@ -687,6 +707,27 @@
>>   }
>> 
>>   /**
>> + * Resizes a new block of native memory for DirectByteBuffers, to 
the
>> + * given size in bytes.  The contents of the new block past the 
size
>> of
>> + * the old block are uninitialized; they will generally be 
garbage.
>> The
>> + * resulting native pointer will be zero if and only if the 
requested
>> size
>> + * is zero.  The resulting native pointer will be aligned for all
>> value
>> + * types.  Dispose of this memory by calling {@link 
#freeDBBMemory},
>> or
>> + * resize it with {@link #reallocateDBBMemory}.  The address 
passed
>> to
>> + * this method may be null, in which case an allocation will be
>> performed.
>> + *
>> + * @throws RuntimeException if the size is negative or too large
>> + *  for the native size_t type
>> + *
>> + * @throws OutOfMemoryError if the allocation is refused by the
>> system
>> + *
>> + * @see #allocateDBBMemory
>> + */
>> +public long reallocateDBBMemory(long address, long bytes) {
>> +return reallocateMemory(address, bytes);
>> +}
>> +
>> +/**
>>* Sets all bytes in a given bl

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[Thomas Stüfe]

On Wed, Feb 14, 2018 at 1:53 PM, David Holmes 
wrote:

> On 14/02/2018 10:43 PM, David Holmes wrote:
>
>> Adding in core-libs-dev as there's nothing related to hotspot directly
>> here.
>>
>
> Correction, this is of course leading to a proposed change in hotspot to
> implement the new Unsafe methods and perform the native memory tracking. Of
> course we already have NMT so the obvious question is how this will fit in
> with NMT?
>
>
I thought Unsafe.allocateMemory is served by hotspot os::malloc(), is it
not? So, allocations should show up in NMT with "Unsafe_AllocateMemory0".

..Thomas



> David
>
>
> David
>>
>> On 14/02/2018 9:32 PM, Adam Farley8 wrote:
>>
>>> Hi All,
>>>
>>> Currently, diagnostic core files generated from OpenJDK seem to lump all
>>> of the
>>> native memory usages together, making it near-impossible for someone to
>>> figure
>>> out *what* is using all that memory in the event of a memory leak.
>>>
>>> The OpenJ9 VM has a feature which allows it to track the allocation of
>>> native
>>> memory for Direct Byte Buffers (DBBs), and to supply that information
>>> into
>>> the
>>> cores when they are generated. This makes it a *lot* easier to find out
>>> what is using
>>> all that native memory, making memory leak resolution less like some dark
>>> art, and
>>> more like logical debugging.
>>>
>>> To use this feature, there is a native method referenced in Unsafe.java.
>>> To open
>>> up this feature so that any VM can make use of it, the java code below
>>> sets the
>>> stage for it. This change starts letting people call DBB-specific methods
>>> when
>>> allocating native memory, and getting into the habit of using it.
>>>
>>> Thoughts?
>>>
>>> Best Regards
>>>
>>> Adam Farley
>>>
>>> P.S. Code:
>>>
>>> diff --git
>>> a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> --- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> +++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> @@ -85,7 +85,7 @@
>>>   // Paranoia
>>>   return;
>>>   }
>>> -UNSAFE.freeMemory(address);
>>> +UNSAFE.freeDBBMemory(address);
>>>   address = 0;
>>>   Bits.unreserveMemory(size, capacity);
>>>   }
>>> @@ -118,7 +118,7 @@
>>>   long base = 0;
>>>   try {
>>> -base = UNSAFE.allocateMemory(size);
>>> +base = UNSAFE.allocateDBBMemory(size);
>>>   } catch (OutOfMemoryError x) {
>>>   Bits.unreserveMemory(size, cap);
>>>   throw x;
>>> diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> --- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> +++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> @@ -632,6 +632,26 @@
>>>   }
>>>   /**
>>> + * Allocates a new block of native memory for DirectByteBuffers, of
>>> the
>>> + * given size in bytes.  The contents of the memory are
>>> uninitialized;
>>> + * they will generally be garbage.  The resulting native pointer
>>> will
>>> + * never be zero, and will be aligned for all value types.  Dispose
>>> of
>>> + * this memory by calling {@link #freeDBBMemory} or resize it with
>>> + * {@link #reallocateDBBMemory}.
>>> + *
>>> + * @throws RuntimeException if the size is negative or too large
>>> + *  for the native size_t type
>>> + *
>>> + * @throws OutOfMemoryError if the allocation is refused by the
>>> system
>>> + *
>>> + * @see #getByte(long)
>>> + * @see #putByte(long, byte)
>>> + */
>>> +public long allocateDBBMemory(long bytes) {
>>> +return allocateMemory(bytes);
>>> +}
>>> +
>>> +/**
>>>* Resizes a new block of native memory, to the given size in
>>> bytes.
>>> The
>>>* contents of the new block past the size of the old block are
>>>* uninitialized; they will generally be garbage.  The resulting
>>> native
>>> @@ -687,6 +707,27 @@
>>>   }
>>>   /**
>>> + * Resizes a new block of native memory for DirectByteBuffers, to
>>> the
>>> + * given size in bytes.  The contents of the new block past the size
>>> of
>>> + * the old block are uninitialized; they will generally be garbage.
>>> The
>>> + * resulting native pointer will be zero if and only if the
>>> requested
>>> size
>>> + * is zero.  The resulting native pointer will be aligned for all
>>> value
>>> + * types.  Dispose of this memory by calling {@link #freeDBBMemory},
>>> or
>>> + * resize it with {@link #reallocateDBBMemory}.  The address passed
>>> to
>>> + * this method may be null, in which case an allocation will be
>>> performed.
>>> + *
>>> + * @throws RuntimeException if the size is negative or too large
>>> +

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[Adam Farley8]

>> Adding in core-libs-dev as there's nothing related to hotspot directly 
>> here.
>
> Correction, this is of course leading to a proposed change in hotspot to 

> implement the new Unsafe methods and perform the native memory tracking. 


Hah, I wrote the same thing in a parallel reply. Jinx. :)

- Adam

> Of course we already have NMT so the obvious question is how this will 
> fit in with NMT?
>
> David

It will add granularity to Native Memory Tracking, allowing people
to tell, at a glance, how much of the allocated native memory has been
used for Direct Byte Buffers. This makes native memory OOM
debugging easier.

Think of it as an NMT upgrade.

Here's an example of what the output should look like:

https://developer.ibm.com/answers/questions/288697/why-does-nativememinfo-in-javacore-show-incorrect.html?sort=oldest

- Adam

>
>> David
>> 
>> On 14/02/2018 9:32 PM, Adam Farley8 wrote:
>>> Hi All,
>>>
>>> Currently, diagnostic core files generated from OpenJDK seem to lump 
all
>>> of the
>>> native memory usages together, making it near-impossible for someone 
to
>>> figure
>>> out *what* is using all that memory in the event of a memory leak.
>>>
>>> The OpenJ9 VM has a feature which allows it to track the allocation of
>>> native
>>> memory for Direct Byte Buffers (DBBs), and to supply that information 
>>> into
>>> the
>>> cores when they are generated. This makes it a *lot* easier to find 
out
>>> what is using
>>> all that native memory, making memory leak resolution less like some 
dark
>>> art, and
>>> more like logical debugging.
>>>
>>> To use this feature, there is a native method referenced in 
Unsafe.java.
>>> To open
>>> up this feature so that any VM can make use of it, the java code below
>>> sets the
>>> stage for it. This change starts letting people call DBB-specific 
methods
>>> when
>>> allocating native memory, and getting into the habit of using it.
>>>
>>> Thoughts?
>>>
>>> Best Regards
>>>
>>> Adam Farley
>>>
>>> P.S. Code:
>>>
>>> diff --git
>>> a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> --- 
a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> +++ 
b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
>>> @@ -85,7 +85,7 @@
>>>   // Paranoia
>>>   return;
>>>   }
>>> -UNSAFE.freeMemory(address);
>>> +UNSAFE.freeDBBMemory(address);
>>>   address = 0;
>>>   Bits.unreserveMemory(size, capacity);
>>>   }
>>> @@ -118,7 +118,7 @@
>>>   long base = 0;
>>>   try {
>>> -base = UNSAFE.allocateMemory(size);
>>> +base = UNSAFE.allocateDBBMemory(size);
>>>   } catch (OutOfMemoryError x) {
>>>   Bits.unreserveMemory(size, cap);
>>>   throw x;
>>> diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> --- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> +++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
>>> @@ -632,6 +632,26 @@
>>>   }
>>>   /**
>>> + * Allocates a new block of native memory for DirectByteBuffers, 
of
>>> the
>>> + * given size in bytes.  The contents of the memory are
>>> uninitialized;
>>> + * they will generally be garbage.  The resulting native pointer 
>>> will
>>> + * never be zero, and will be aligned for all value types.  
Dispose
>>> of
>>> + * this memory by calling {@link #freeDBBMemory} or resize it 
with
>>> + * {@link #reallocateDBBMemory}.
>>> + *
>>> + * @throws RuntimeException if the size is negative or too large
>>> + *  for the native size_t type
>>> + *
>>> + * @throws OutOfMemoryError if the allocation is refused by the
>>> system
>>> + *
>>> + * @see #getByte(long)
>>> + * @see #putByte(long, byte)
>>> + */
>>> +public long allocateDBBMemory(long bytes) {
>>> +return allocateMemory(bytes);
>>> +}
>>> +
>>> +/**
>>>* Resizes a new block of native memory, to the given size in 
>>> bytes.
>>> The
>>>* contents of the new block past the size of the old block are
>>>* uninitialized; they will generally be garbage.  The resulting
>>> native
>>> @@ -687,6 +707,27 @@
>>>   }
>>>   /**
>>> + * Resizes a new block of native memory for DirectByteBuffers, to 

>>> the
>>> + * given size in bytes.  The contents of the new block past the 
size
>>> of
>>> + * the old block are uninitialized; they will generally be 
garbage.
>>> The
>>> + * resulting native pointer will be zero if and only if the 
>>> requested
>>> size
>>> + * is zero.  The resulting native pointer will be aligned for all
>>> value
>>> + * types.  Dispose of this memory by calling {@link 
#freeDBBMemory},
>>> or
>>> +   

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[Thomas Stüfe]

On Wed, Feb 14, 2018 at 2:32 PM, Adam Farley8 
wrote:

> >> Adding in core-libs-dev as there's nothing related to hotspot directly
> >> here.
> >
> > Correction, this is of course leading to a proposed change in hotspot to
>
> > implement the new Unsafe methods and perform the native memory tracking.
>
>
> Hah, I wrote the same thing in a parallel reply. Jinx. :)
>
> - Adam
>
> > Of course we already have NMT so the obvious question is how this will
> > fit in with NMT?
> >
> > David
>
> It will add granularity to Native Memory Tracking, allowing people
> to tell, at a glance, how much of the allocated native memory has been
> used for Direct Byte Buffers. This makes native memory OOM
> debugging easier.
>
> Think of it as an NMT upgrade.
>
> Here's an example of what the output should look like:
>
> https://developer.ibm.com/answers/questions/288697/why-
> does-nativememinfo-in-javacore-show-incorrect.html?sort=oldest
>
> - Adam
>
>
I think NMT walks the stack, so we should get allocation points grouped by
call stacks. Provided we have symbols loaded for the native library using
Unsafe.allocateMemory(), this should give us too a fine granularity. But I
have not yet tested this in practice. Maybe Zhengyu knows more.

..Thomas


> >
> >> David
> >>
> >> On 14/02/2018 9:32 PM, Adam Farley8 wrote:
> >>> Hi All,
> >>>
> >>> Currently, diagnostic core files generated from OpenJDK seem to lump
> all
> >>> of the
> >>> native memory usages together, making it near-impossible for someone
> to
> >>> figure
> >>> out *what* is using all that memory in the event of a memory leak.
> >>>
> >>> The OpenJ9 VM has a feature which allows it to track the allocation of
> >>> native
> >>> memory for Direct Byte Buffers (DBBs), and to supply that information
> >>> into
> >>> the
> >>> cores when they are generated. This makes it a *lot* easier to find
> out
> >>> what is using
> >>> all that native memory, making memory leak resolution less like some
> dark
> >>> art, and
> >>> more like logical debugging.
> >>>
> >>> To use this feature, there is a native method referenced in
> Unsafe.java.
> >>> To open
> >>> up this feature so that any VM can make use of it, the java code below
> >>> sets the
> >>> stage for it. This change starts letting people call DBB-specific
> methods
> >>> when
> >>> allocating native memory, and getting into the habit of using it.
> >>>
> >>> Thoughts?
> >>>
> >>> Best Regards
> >>>
> >>> Adam Farley
> >>>
> >>> P.S. Code:
> >>>
> >>> diff --git
> >>> a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> >>> b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> >>> ---
> a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> >>> +++
> b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> >>> @@ -85,7 +85,7 @@
> >>>   // Paranoia
> >>>   return;
> >>>   }
> >>> -UNSAFE.freeMemory(address);
> >>> +UNSAFE.freeDBBMemory(address);
> >>>   address = 0;
> >>>   Bits.unreserveMemory(size, capacity);
> >>>   }
> >>> @@ -118,7 +118,7 @@
> >>>   long base = 0;
> >>>   try {
> >>> -base = UNSAFE.allocateMemory(size);
> >>> +base = UNSAFE.allocateDBBMemory(size);
> >>>   } catch (OutOfMemoryError x) {
> >>>   Bits.unreserveMemory(size, cap);
> >>>   throw x;
> >>> diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> >>> b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> >>> --- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> >>> +++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> >>> @@ -632,6 +632,26 @@
> >>>   }
> >>>   /**
> >>> + * Allocates a new block of native memory for DirectByteBuffers,
> of
> >>> the
> >>> + * given size in bytes.  The contents of the memory are
> >>> uninitialized;
> >>> + * they will generally be garbage.  The resulting native pointer
> >>> will
> >>> + * never be zero, and will be aligned for all value types.
> Dispose
> >>> of
> >>> + * this memory by calling {@link #freeDBBMemory} or resize it
> with
> >>> + * {@link #reallocateDBBMemory}.
> >>> + *
> >>> + * @throws RuntimeException if the size is negative or too large
> >>> + *  for the native size_t type
> >>> + *
> >>> + * @throws OutOfMemoryError if the allocation is refused by the
> >>> system
> >>> + *
> >>> + * @see #getByte(long)
> >>> + * @see #putByte(long, byte)
> >>> + */
> >>> +public long allocateDBBMemory(long bytes) {
> >>> +return allocateMemory(bytes);
> >>> +}
> >>> +
> >>> +/**
> >>>* Resizes a new block of native memory, to the given size in
> >>> bytes.
> >>> The
> >>>* contents of the new block past the size of the old block are
> >>>* uninitialized; they will generally be garbage.

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

[Adam Farley8]

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.

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://bugs.openjdk.java.net/browse/JDK-8190187

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

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

[Alan Bateman]

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). There is also text to be written for the JNI 
spec if this proposal goes ahead.


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, ...).



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


-Alan

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

[mark . reinhold]

2018/2/14 2:20:22 -0800, volker.simo...@gmail.com:
> can I please get a review for the following tiny fix:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
> https://bugs.openjdk.java.net/browse/JDK-8197927
> 
> The new Java 10 specification makes the 'java.vendor.version' property
> mandatory [1] but the current implementations doesn't allow to set it
> to an empty string.

This is a bug in the specification, not the implementation.  As I just
wrote in a comment on 8197927:

JEP 322 expresses the intended behavior: If `--with-vendor-version` is
not specified at build time then the `java.vendor.version` property has
no value.  (That's different from the empty string, which is a value.)
The bug is in the specification, which should be revised so as not to
require that this property have a value.  Fixing the spec is not
critical for 10; I suggest downgrading 8197927 to P2 and targeting 11.

- Mark

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[Zhengyu Gu]

On 02/14/2018 08:16 AM, Thomas Stüfe wrote:

On Wed, Feb 14, 2018 at 1:53 PM, David Holmes 
wrote:


On 14/02/2018 10:43 PM, David Holmes wrote:


Adding in core-libs-dev as there's nothing related to hotspot directly
here.



Correction, this is of course leading to a proposed change in hotspot to
implement the new Unsafe methods and perform the native memory tracking. Of
course we already have NMT so the obvious question is how this will fit in
with NMT?



I thought Unsafe.allocateMemory is served by hotspot os::malloc(), is it
not? So, allocations should show up in NMT with "Unsafe_AllocateMemory0".


Could use another category? to make it earlier to identify.

Thanks,

-Zhengyu



..Thomas




David


David


On 14/02/2018 9:32 PM, Adam Farley8 wrote:


Hi All,

Currently, diagnostic core files generated from OpenJDK seem to lump all
of the
native memory usages together, making it near-impossible for someone to
figure
out *what* is using all that memory in the event of a memory leak.

The OpenJ9 VM has a feature which allows it to track the allocation of
native
memory for Direct Byte Buffers (DBBs), and to supply that information
into
the
cores when they are generated. This makes it a *lot* easier to find out
what is using
all that native memory, making memory leak resolution less like some dark
art, and
more like logical debugging.

To use this feature, there is a native method referenced in Unsafe.java.
To open
up this feature so that any VM can make use of it, the java code below
sets the
stage for it. This change starts letting people call DBB-specific methods
when
allocating native memory, and getting into the habit of using it.

Thoughts?

Best Regards

Adam Farley

P.S. Code:

diff --git
a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
--- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
+++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
@@ -85,7 +85,7 @@
   // Paranoia
   return;
   }
-UNSAFE.freeMemory(address);
+UNSAFE.freeDBBMemory(address);
   address = 0;
   Bits.unreserveMemory(size, capacity);
   }
@@ -118,7 +118,7 @@
   long base = 0;
   try {
-base = UNSAFE.allocateMemory(size);
+base = UNSAFE.allocateDBBMemory(size);
   } catch (OutOfMemoryError x) {
   Bits.unreserveMemory(size, cap);
   throw x;
diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
--- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
+++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
@@ -632,6 +632,26 @@
   }
   /**
+ * Allocates a new block of native memory for DirectByteBuffers, of
the
+ * given size in bytes.  The contents of the memory are
uninitialized;
+ * they will generally be garbage.  The resulting native pointer
will
+ * never be zero, and will be aligned for all value types.  Dispose
of
+ * this memory by calling {@link #freeDBBMemory} or resize it with
+ * {@link #reallocateDBBMemory}.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #getByte(long)
+ * @see #putByte(long, byte)
+ */
+public long allocateDBBMemory(long bytes) {
+return allocateMemory(bytes);
+}
+
+/**
* Resizes a new block of native memory, to the given size in
bytes.
The
* contents of the new block past the size of the old block are
* uninitialized; they will generally be garbage.  The resulting
native
@@ -687,6 +707,27 @@
   }
   /**
+ * Resizes a new block of native memory for DirectByteBuffers, to
the
+ * given size in bytes.  The contents of the new block past the size
of
+ * the old block are uninitialized; they will generally be garbage.
The
+ * resulting native pointer will be zero if and only if the
requested
size
+ * is zero.  The resulting native pointer will be aligned for all
value
+ * types.  Dispose of this memory by calling {@link #freeDBBMemory},
or
+ * resize it with {@link #reallocateDBBMemory}.  The address passed
to
+ * this method may be null, in which case an allocation will be
performed.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #allocateDBBMemory
+ */
+public long reallocateDBBMemory(long address, long bytes) {
+return reallocateMemory(address, bytes);
+}
+
+/**
* Sets all

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[Zhengyu Gu]

Think of it as an NMT upgrade.

Here's an example of what the output should look like:

https://developer.ibm.com/answers/questions/288697/why-
does-nativememinfo-in-javacore-show-incorrect.html?sort=oldest

- Adam



I think NMT walks the stack, so we should get allocation points grouped by
call stacks. Provided we have symbols loaded for the native library using
Unsafe.allocateMemory(), this should give us too a fine granularity. But I
have not yet tested this in practice. Maybe Zhengyu knows more.


Quick test shows this call site:

[0x7f8558b26243] Unsafe_AllocateMemory0+0x93
[0x7f8537b085cb]
 (malloc=2KB type=Internal #1)

I will take a look why there is a frame not decoded.

Thanks,

-Zhengyu





..Thomas





David

On 14/02/2018 9:32 PM, Adam Farley8 wrote:

Hi All,

Currently, diagnostic core files generated from OpenJDK seem to lump

all

of the
native memory usages together, making it near-impossible for someone

to

figure
out *what* is using all that memory in the event of a memory leak.

The OpenJ9 VM has a feature which allows it to track the allocation of
native
memory for Direct Byte Buffers (DBBs), and to supply that information
into
the
cores when they are generated. This makes it a *lot* easier to find

out

what is using
all that native memory, making memory leak resolution less like some

dark

art, and
more like logical debugging.

To use this feature, there is a native method referenced in

Unsafe.java.

To open
up this feature so that any VM can make use of it, the java code below
sets the
stage for it. This change starts letting people call DBB-specific

methods

when
allocating native memory, and getting into the habit of using it.

Thoughts?

Best Regards

Adam Farley

P.S. Code:

diff --git
a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
---

a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template

+++

b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template

@@ -85,7 +85,7 @@
   // Paranoia
   return;
   }
-UNSAFE.freeMemory(address);
+UNSAFE.freeDBBMemory(address);
   address = 0;
   Bits.unreserveMemory(size, capacity);
   }
@@ -118,7 +118,7 @@
   long base = 0;
   try {
-base = UNSAFE.allocateMemory(size);
+base = UNSAFE.allocateDBBMemory(size);
   } catch (OutOfMemoryError x) {
   Bits.unreserveMemory(size, cap);
   throw x;
diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
--- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
+++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
@@ -632,6 +632,26 @@
   }
   /**
+ * Allocates a new block of native memory for DirectByteBuffers,

of

the
+ * given size in bytes.  The contents of the memory are
uninitialized;
+ * they will generally be garbage.  The resulting native pointer
will
+ * never be zero, and will be aligned for all value types.

Dispose

of
+ * this memory by calling {@link #freeDBBMemory} or resize it

with

+ * {@link #reallocateDBBMemory}.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #getByte(long)
+ * @see #putByte(long, byte)
+ */
+public long allocateDBBMemory(long bytes) {
+return allocateMemory(bytes);
+}
+
+/**
* Resizes a new block of native memory, to the given size in
bytes.
The
* contents of the new block past the size of the old block are
* uninitialized; they will generally be garbage.  The resulting
native
@@ -687,6 +707,27 @@
   }
   /**
+ * Resizes a new block of native memory for DirectByteBuffers, to



the
+ * given size in bytes.  The contents of the new block past the

size

of
+ * the old block are uninitialized; they will generally be

garbage.

The
+ * resulting native pointer will be zero if and only if the
requested
size
+ * is zero.  The resulting native pointer will be aligned for all
value
+ * types.  Dispose of this memory by calling {@link

#freeDBBMemory},

or
+ * resize it with {@link #reallocateDBBMemory}.  The address

passed

to
+ * this method may be null, in which case an allocation will be
performed.
+ *
+ * @throws RuntimeException if the size is negative or too large
+ *  for the native size_t type
+ *
+ * @throws OutOfMemoryError if the allocation is refused by the
system
+ *
+ * @see #allocateDBBMemory
+ */
+public long reallocateDBBMemory(long address, long bytes) {
+

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native MemoryUsage for Direct Byte Buffers

[Bernd Eckenfels]

Maybe instead adding a „allocation request type“ argment to allocate Memory? 
(and wrap it with a typed allocator in the buffer interfacessomewhere?) the 
„DBB“ part Looks especially cryptic.  We have similiar concepts for NMT in the 
native Code.

Besides I mentioned a while back that the JMX part of the memory Accounting 
could be improved as well. Those two could probably be unified. This is where I 
see most Troubleshooting activities struggle.

Gruss
Bernd
-- 
http://bernd.eckenfels.net

Von: David Holmes
Gesendet: Mittwoch, 14. Februar 2018 16:44
An: Adam Farley8; hotspot-...@openjdk.java.net; core-libs-dev Libs
Betreff: Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native 
MemoryUsage for Direct Byte Buffers

Adding in core-libs-dev as there's nothing related to hotspot directly here.

David

On 14/02/2018 9:32 PM, Adam Farley8 wrote:
> Hi All,
> 
> Currently, diagnostic core files generated from OpenJDK seem to lump all
> of the
> native memory usages together, making it near-impossible for someone to
> figure
> out *what* is using all that memory in the event of a memory leak.
> 
> The OpenJ9 VM has a feature which allows it to track the allocation of
> native
> memory for Direct Byte Buffers (DBBs), and to supply that information into
> the
> cores when they are generated. This makes it a *lot* easier to find out
> what is using
> all that native memory, making memory leak resolution less like some dark
> art, and
> more like logical debugging.
> 
> To use this feature, there is a native method referenced in Unsafe.java.
> To open
> up this feature so that any VM can make use of it, the java code below
> sets the
> stage for it. This change starts letting people call DBB-specific methods
> when
> allocating native memory, and getting into the habit of using it.
> 
> Thoughts?
> 
> Best Regards
> 
> Adam Farley
> 
> P.S. Code:
> 
> diff --git
> a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> --- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> +++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
> @@ -85,7 +85,7 @@
>   // Paranoia
>   return;
>   }
> -UNSAFE.freeMemory(address);
> +UNSAFE.freeDBBMemory(address);
>   address = 0;
>   Bits.unreserveMemory(size, capacity);
>   }
> @@ -118,7 +118,7 @@
>   
>   long base = 0;
>   try {
> -base = UNSAFE.allocateMemory(size);
> +base = UNSAFE.allocateDBBMemory(size);
>   } catch (OutOfMemoryError x) {
>   Bits.unreserveMemory(size, cap);
>   throw x;
> diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> --- a/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> +++ b/src/java.base/share/classes/jdk/internal/misc/Unsafe.java
> @@ -632,6 +632,26 @@
>   }
>   
>   /**
> + * Allocates a new block of native memory for DirectByteBuffers, of
> the
> + * given size in bytes.  The contents of the memory are
> uninitialized;
> + * they will generally be garbage.  The resulting native pointer will
> + * never be zero, and will be aligned for all value types.  Dispose
> of
> + * this memory by calling {@link #freeDBBMemory} or resize it with
> + * {@link #reallocateDBBMemory}.
> + *
> + * @throws RuntimeException if the size is negative or too large
> + *  for the native size_t type
> + *
> + * @throws OutOfMemoryError if the allocation is refused by the
> system
> + *
> + * @see #getByte(long)
> + * @see #putByte(long, byte)
> + */
> +public long allocateDBBMemory(long bytes) {
> +return allocateMemory(bytes);
> +}
> +
> +/**
>* Resizes a new block of native memory, to the given size in bytes.
> The
>* contents of the new block past the size of the old block are
>* uninitialized; they will generally be garbage.  The resulting
> native
> @@ -687,6 +707,27 @@
>   }
>   
>   /**
> + * Resizes a new block of native memory for DirectByteBuffers, to the
> + * given size in bytes.  The contents of the new block past the size
> of
> + * the old block are uninitialized; they will generally be garbage.
> The
> + * resulting native pointer will be zero if and only if the requested
> size
> + * is zero.  The resulting native pointer will be aligned for all
> value
> + * types.  Dispose of this memory by calling {@link #freeDBBMemory},
> or
> + * resize it with {@link #reallocateDBBMemory}.  The address passed
> to
> + * this method may be null, in which case an allocation will be
> performed.
> + *
> + * @throws RuntimeException if the size is negative or too large
> + * 

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

[Volker Simonis]

On Wed, Feb 14, 2018 at 4:26 PM,   wrote:
> 2018/2/14 2:20:22 -0800, volker.simo...@gmail.com:
>> can I please get a review for the following tiny fix:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/
>> https://bugs.openjdk.java.net/browse/JDK-8197927
>>
>> The new Java 10 specification makes the 'java.vendor.version' property
>> mandatory [1] but the current implementations doesn't allow to set it
>> to an empty string.
>
> This is a bug in the specification, not the implementation.  As I just
> wrote in a comment on 8197927:
>
> JEP 322 expresses the intended behavior: If `--with-vendor-version` is
> not specified at build time then the `java.vendor.version` property has
> no value.  (That's different from the empty string, which is a value.)
> The bug is in the specification, which should be revised so as not to
> require that this property have a value.  Fixing the spec is not
> critical for 10; I suggest downgrading 8197927 to P2 and targeting 11.
>

What you mean by "not require the 'java.vendor.version' property to
have a value"? We can not set a system property to NULL because
System.setProperty() will throw a NPE in that case. So either the
specification requires 'java.vendor.version' as mandatory (as it is
now) in which case it does need to have a value different from NULL.
Or we change the specification such that 'java.vendor.version' isn't a
mandatory property any more. But it is not possible to have
'java.vendor.version' as mandatory property with "no value" as
envisioned by the JEP.

Do you propose to change the specification such that
'java.vendor.version' isn't a mandatory property any more? That would
be fine for me and I agree that we could target this bug to 11 if
that's what you propose. We could actually list "java.vendor.version"
under "Implementation Note:" which contains some additional,
non-mandatory properties.

> - Mark

Re: RFR: 8197893: Mistaken type check in CheckedEntrySet.toArray

[Paul Sandoz]

> On Feb 13, 2018, at 5:33 PM, Martin Buchholz  wrote:
> 
> http://cr.openjdk.java.net/~martin/webrevs/jdk/CheckedEntrySet-toArray/
> https://bugs.openjdk.java.net/browse/JDK-8197893

+1

Paul.

Re: RFR JDK-8164278: java.util.Base64.EncOutputStream/DecInputStream is slower than corresponding version in javax.mail package

[Roger Riggs]

Hi Sherman,

I found updates in http://cr.openjdk.java.net/~sherman/8164278/webrev.04

That looks fine.

Typo:  line 1008: "neve"

Roger



On 2/7/2018 10:32 PM, Xueming Shen wrote:

Hi Roger,

Given the concern of the possible incompatible behavior change of over 
reading bytes
from the underlying stream. I decided to give up last proposed changes 
for DecInputStream
for now. With some "minor" cleanup and tuning I still have about 10%+ 
improvement with
various input size sampling. Let's address the decoder stream in 
separate rfe later, if desired.


The updated webrev and the jmh results are at

http://cr.openjdk.java.net/~sherman/8164278/webrev
http://cr.openjdk.java.net/~sherman/8164278/base64.bm

Last version of webrev and corresponding jmh result can be found at
http://cr.openjdk.java.net/~sherman/8164278/webrev.02
http://cr.openjdk.java.net/~sherman/8164278/base64.bm.old

Thanks,
Sherman

On 2/5/18, 6:00 PM, Xueming Shen wrote:

Hi,

Please help review the change for  JDK-8164278.

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

jmh.src: http://cr.openjdk.java.net/~sherman/8164278/Base64BM.java
jmh.result: http://cr.openjdk.java.net/~sherman/8164278/base64.bm

Base64.Decoder.decode0:
    Adopted the "similar" optimization approach we took in 
Base64.Encoder.encode0()
    to add a "fast path" to decode a block of 4-byte units together 
(current implementation
    decodes one single byte per while/loop. The jmh benchmark result 
indicates a big speed
    boost  (those decodeArray/Mime/Url results, from 30% to 2 times 
faster, depends on

    input size).

Base64.Encoder.encode0()
    It appears encode0() was fully optimized in 1.8. Can't get it 
faster :-) Tried to use
    Unsafe.getLong/putLong instead of byte by byte access. But it 
appears the 8-byte
    "vectorization" does not bring us enough speed up, the 
performance is the same as the

    current one. See encode00() at
    http://cr.openjdk.java.net/~sherman/8164278/webrev.00

Base64.Encoder.wrap(OutputStream)/EncOutputStream.write():
    If my memory serves me right, the current implementation was 
under the assumption that
    the underlying output stream probably is buffered (if invoker 
cares). It would be a redundant
    if EncOutputStream buffers bytes again. It appears this is a 
wrong assumption. It is much
    slower to write 4 bytes separately, compared to bundle them 
together in a byte[4] and write
    into underlying, even the underlying output stream is a 
ByteArrayOutputStream.
    Again, the proposed change is to add a fast path loop, as we do 
in encode0(), to decode/
    write a block of 3-byte->4-byte unites. It appears this fast loop 
can help the compiler to

    optimize away some boundary checks, therefor is much faster.
    The jmh result Base64BM.encodeOS suggests the new implementation 
is almost 4 times faster

    and is almost the same as java.mail's stream encoder.

Base64.Decoder.wrap(InputStream)/DecInputStream.read():
    Same as the approach we take for decode0(), to add a fast path 
decode block of 4-byte unites

    together.
    The jmh result Base64BM.decodeOS (the name probably should be 
decodeIS, for InputStream,
    but anyway...) shows the proposed one is 4 times faster than the 
existing impl and double

    the  java.mail (Base64BM.decodeOS_javamail) implementation.

    However, there is a side-effect of adding a buffering mechanism 
into DecInputStream. The
    current implementation read bytes from the underlying stream one 
by one, it never reads
    more bytes than it needs, which means it should/is supposed to 
just stop at the last byte

    that it needs to decode, when there is "=" present in the stream.
    With buffering, it's possible more bytes (after "=", which 
indicates "end of base64 stream") might
    be read/consumed in and buffered.  A concern? if this is indeed a 
concern, the only alternative
    might be to add a separate method to support this 
"faster-buffered-decoder"?


Thanks,
Sherman

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

[yumin qi]

Hi, Alan

  Thanks. Updated on same link
   http://cr.openjdk.java.net/~minqi/8194154/webrev1/
  as your recommendation.

Yumin

On Wed, Feb 14, 2018 at 4:42 AM, Alan Bateman 
wrote:

> On 14/02/2018 01:23, yumin qi wrote:
>
>> Hi,
>>
>>   I have update the webrev:
>> http://cr.openjdk.java.net/~minqi/8194154/webrev1/ <
>> http://cr.openjdk.java.net/%7Eminqi/8194154/webrev1/>
>>
>>   In this version, as suggested by Alan(thanks!), property of "user.dir"
>> is cached and behave like it is 'read only'. The change made to *inux as
>> well as windows. Since property of "user.dir" is cached, any changes via
>> property setting for it has no effect.
>>
>> This looks much better but you've removed a permission check from the
> Windows getUserPath implementation. That code the same permission check as
> the resolve method in the Unix implementation.
>
> I think the test needs works. The simplest would be to call
> getCanonicalFile before changing the system property, then call it after
> and check that you an equal result. Also no need to hack the Properties
> object, you can use System.setProperty instead.
>
> -Alan
>
>
>

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

[Roger Riggs]

Hi,

In the test, the message can be improved; though the exception should 
never occur

it will be misleading if it does.
Instead:
    "Changing property user.dir should have no effect on the 
getCanonicalPath"


It is cleaner to throw AssertionError for the test failure.
Throwing FileSystemException makes it look like it came from inside the 
file system implementation.


Regards, Roger

On 2/13/2018 6:45 PM, yumin qi wrote:

Alan,

   In fact, if property "user.dir" is cached, the problem will go away
without any changes to native or UnixFileSystem.java, since it won't
canonicalize the string supplied from user. The output will be
   getProperty("user.dir") + "/" + file. (The result is not as expected,
user can not change the property, means user has to work around the problem
to reach their targeted directory to do things like loading classes)

   Any change to "user.dir" in java program does not have any effect.
   For Windows:

--- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java Fri Feb 02
10:32:59 2018 -0800
+++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java Tue Feb 13
23:40:21 2018 +
@@ -43,12 +43,14 @@
  private final char slash;
  private final char altSlash;
  private final char semicolon;
+private final String userDir;

  public WinNTFileSystem() {
  Properties props = GetPropertyAction.privilegedGetProperties();
  slash = props.getProperty("file.separator").charAt(0);
  semicolon = props.getProperty("path.separator").charAt(0);
  altSlash = (this.slash == '\\') ? '/' : '\\';
+userDir = props.getProperty("user.dir");
  }

  private boolean isSlash(char c) {
@@ -347,7 +349,7 @@
  private String getUserPath() {
  /* For both compatibility and security,
 we must look this up every time */
-return normalize(System.getProperty("user.dir"));
+return normalize(userDir);
  }

  private String getDrive(String path) {

   Does it need normalize(userDir) in getUserPath()? I think we need here.

   I will update the webrev and send for another round of review: the change
in native will be reverted, and made changes to UnixFileSystem.java.


Thanks
Yumin


On Mon, Feb 12, 2018 at 3:09 AM, Alan Bateman 
wrote:


On 09/02/2018 18:01, Alan Bateman wrote:


:

I'll study the patch you have but I think we also need to create issues
to get us to the point where changing this system property in a running VM
doesn't impact running code.


Looking at it again, I think we should change java.io.UnixFileSystem (and
the Windows equivalent) to cache the value of user.dir to avoid difficult
to diagnose issues with bad code changing the value of this property in a
running VM. This should reduce the issue down to cases where user.dir is
changed on the command line (never supported either of course) to a value
that is not "/" but has trailing or duplicate slashes.

When reduced down then the alternatives are to change the native
canonicalize method as you have done or alternatively do it once at
UnixFileSystem initialization time so that canonicalize does not have to
deal with this case. The former would require changing the description of
the function (it currently reads "The input path is assumed to contain no
duplicate slashes"), the latter avoids any changes to the native
implementation.

-Alan


diff -r 0937e5f799df src/java.base/unix/classes/jav
a/io/UnixFileSystem.java
--- a/src/java.base/unix/classes/java/io/UnixFileSystem.javaSat Feb
10 07:06:16 2018 -0500
+++ b/src/java.base/unix/classes/java/io/UnixFileSystem.javaMon Feb
12 10:49:40 2018 +
@@ -34,12 +34,14 @@
  private final char slash;
  private final char colon;
  private final String javaHome;
+private final String userDir;

  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 = props.getProperty("user.dir");
  }


@@ -128,7 +130,11 @@

  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());
  }

  // Caches for canonicalization results to improve startup performance.

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

[Brian Burkhalter]

Hello,

In the test you have
/* @test requires (os.family == "linux") | (os.family == "mac") | (os.family == 
"solaris") | (os.family == "aix")
   @bug 8194154
   @summary Test parsing path with double slashes on unix like platforms.
 */
but I think you need to have the @requires annotation on a separate line for 
example as:
/* @test
 * @requires (os.family == "linux") | (os.family == "mac”)
 * | (os.family == "solaris") | (os.family == "aix")
 * @bug 8194154
 * @summary Test parsing path with double slashes on unix like platforms.
 */
Thanks,

Brian

On Feb 14, 2018, at 10:48 AM, yumin qi  wrote:

>  Thanks. Updated on same link
>   http://cr.openjdk.java.net/~minqi/8194154/webrev1/
>  as your recommendation.

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

[mandy chung]

On 2/14/18 1:58 AM, Peter Levart wrote:


I take back this claim. Of course the the following race is possible:

- Thread1: calls runAllFinalizers and takes a Finalizer from 
'unprocessed' list.
- Thread2: takee the same Finalizer instance from ReferenceQueue and 
calls runFinalizer()
- Thread1: calls runFinalizer() with the same instance for the 2nd 
time now.


runAllFinalizers is invoked during shutdown when 
System.runFinalizersOnExit has been called.


I have been wanting to remove System::runFinalizersOnExit [1] which is 
the culprit of causing this complicated handling.   Probably time to 
remove it in 11?


Mandy

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

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

[sangheon.kim]

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

Thanks,
Sangheon

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

[Martin Buchholz]

Indeed I had suppressed all memory of runFinalizersOnExit.  (Sorry about
that.)
I support removing it in jdk11.
Mandy, would you like to file the CSR?

On Wed, Feb 14, 2018 at 1:15 PM, mandy chung  wrote:

>
>
> On 2/14/18 1:58 AM, Peter Levart wrote:
>
>
> I take back this claim. Of course the the following race is possible:
>
> - Thread1: calls runAllFinalizers and takes a Finalizer from 'unprocessed'
> list.
> - Thread2: takee the same Finalizer instance from ReferenceQueue and calls
> runFinalizer()
> - Thread1: calls runFinalizer() with the same instance for the 2nd time
> now.
>
>
> runAllFinalizers is invoked during shutdown when
> System.runFinalizersOnExit has been called.
>
> I have been wanting to remove System::runFinalizersOnExit [1] which is the
> culprit of causing this complicated handling.   Probably time to remove it
> in 11?
>
> Mandy
>
> [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-
> January/031041.html
>

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

[mandy chung]

On 2/14/18 1:54 PM, Martin Buchholz wrote:
Indeed I had suppressed all memory of runFinalizersOnExit. (Sorry 
about that.)

I support removing it in jdk11.


Great.


Mandy, would you like to file the CSR?


Yes I plan to do that for:
    https://bugs.openjdk.java.net/browse/JDK-4240589

Mandy


On Wed, Feb 14, 2018 at 1:15 PM, mandy chung > wrote:




On 2/14/18 1:58 AM, Peter Levart wrote:


I take back this claim. Of course the the following race is
possible:

- Thread1: calls runAllFinalizers and takes a Finalizer from
'unprocessed' list.
- Thread2: takee the same Finalizer instance from ReferenceQueue
and calls runFinalizer()
- Thread1: calls runFinalizer() with the same instance for the
2nd time now.


runAllFinalizers is invoked during shutdown when
System.runFinalizersOnExit has been called.

I have been wanting to remove System::runFinalizersOnExit [1]
which is the culprit of causing this complicated handling.  
Probably time to remove it in 11?

Mandy

[1]

http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-January/031041.html

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

[yumin qi]

HI, Roger and Brian

  Updated again in same link.
  I could not run jtreg so please have a look if the options are good for
@run

Thanks
Yumin

On Wed, Feb 14, 2018 at 11:13 AM, Roger Riggs 
wrote:

> Hi,
>
> In the test, the message can be improved; though the exception should
> never occur
> it will be misleading if it does.
> Instead:
> "Changing property user.dir should have no effect on the
> getCanonicalPath"
>
> It is cleaner to throw AssertionError for the test failure.
> Throwing FileSystemException makes it look like it came from inside the
> file system implementation.
>
> Regards, Roger
>
>
> On 2/13/2018 6:45 PM, yumin qi wrote:
>
>> Alan,
>>
>>In fact, if property "user.dir" is cached, the problem will go away
>> without any changes to native or UnixFileSystem.java, since it won't
>> canonicalize the string supplied from user. The output will be
>>getProperty("user.dir") + "/" + file. (The result is not as expected,
>> user can not change the property, means user has to work around the
>> problem
>> to reach their targeted directory to do things like loading classes)
>>
>>Any change to "user.dir" in java program does not have any effect.
>>For Windows:
>>
>> --- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java Fri Feb
>> 02
>> 10:32:59 2018 -0800
>> +++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java Tue Feb
>> 13
>> 23:40:21 2018 +
>> @@ -43,12 +43,14 @@
>>   private final char slash;
>>   private final char altSlash;
>>   private final char semicolon;
>> +private final String userDir;
>>
>>   public WinNTFileSystem() {
>>   Properties props = GetPropertyAction.privilegedGetProperties();
>>   slash = props.getProperty("file.separator").charAt(0);
>>   semicolon = props.getProperty("path.separator").charAt(0);
>>   altSlash = (this.slash == '\\') ? '/' : '\\';
>> +userDir = props.getProperty("user.dir");
>>   }
>>
>>   private boolean isSlash(char c) {
>> @@ -347,7 +349,7 @@
>>   private String getUserPath() {
>>   /* For both compatibility and security,
>>  we must look this up every time */
>> -return normalize(System.getProperty("user.dir"));
>> +return normalize(userDir);
>>   }
>>
>>   private String getDrive(String path) {
>>
>>Does it need normalize(userDir) in getUserPath()? I think we need here.
>>
>>I will update the webrev and send for another round of review: the
>> change
>> in native will be reverted, and made changes to UnixFileSystem.java.
>>
>>
>> Thanks
>> Yumin
>>
>>
>> On Mon, Feb 12, 2018 at 3:09 AM, Alan Bateman 
>> wrote:
>>
>> On 09/02/2018 18:01, Alan Bateman wrote:
>>>
>>> :

 I'll study the patch you have but I think we also need to create issues
 to get us to the point where changing this system property in a running
 VM
 doesn't impact running code.

 Looking at it again, I think we should change java.io.UnixFileSystem
>>> (and
>>> the Windows equivalent) to cache the value of user.dir to avoid difficult
>>> to diagnose issues with bad code changing the value of this property in a
>>> running VM. This should reduce the issue down to cases where user.dir is
>>> changed on the command line (never supported either of course) to a value
>>> that is not "/" but has trailing or duplicate slashes.
>>>
>>> When reduced down then the alternatives are to change the native
>>> canonicalize method as you have done or alternatively do it once at
>>> UnixFileSystem initialization time so that canonicalize does not have to
>>> deal with this case. The former would require changing the description of
>>> the function (it currently reads "The input path is assumed to contain no
>>> duplicate slashes"), the latter avoids any changes to the native
>>> implementation.
>>>
>>> -Alan
>>>
>>>
>>> diff -r 0937e5f799df src/java.base/unix/classes/jav
>>> a/io/UnixFileSystem.java
>>> --- a/src/java.base/unix/classes/java/io/UnixFileSystem.javaSat Feb
>>> 10 07:06:16 2018 -0500
>>> +++ b/src/java.base/unix/classes/java/io/UnixFileSystem.javaMon Feb
>>> 12 10:49:40 2018 +
>>> @@ -34,12 +34,14 @@
>>>   private final char slash;
>>>   private final char colon;
>>>   private final String javaHome;
>>> +private final String userDir;
>>>
>>>   public UnixFileSystem() {
>>>   Properties props = GetPropertyAction.privilegedGe
>>> tProperties();
>>>   slash = props.getProperty("file.separator").charAt(0);
>>>   colon = props.getProperty("path.separator").charAt(0);
>>>   javaHome = props.getProperty("java.home");
>>> +userDir = props.getProperty("user.dir");
>>>   }
>>>
>>>
>>> @@ -128,7 +130,11 @@
>>>
>>>   public String resolve(File f) {
>>>   if (isAbsolute(f)) return f.getPath();
>>> -return resolve(System.getProperty("user.dir"), f.getPath());
>>> +SecurityManager sm = System.getSecurityManager(

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

[David Holmes]

That all seems trivially fine.

Thanks,
David

On 15/02/2018 7:45 AM, 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

Thanks,
Sangheon

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

[Brian Burkhalter]

Yumin,

As for the test options you would need to make this change:

--- a/test/jdk/java/io/File/ParseCanonicalPath.java
+++ b/test/jdk/java/io/File/ParseCanonicalPath.java
@@ -25,7 +25,7 @@
   (os.family == "solaris") | (os.family == "aix")
@bug 8194154
@summary Test parsing path with double slashes on unix like platforms.
-   @run -ea main ParseCanonicalPath
+   @run main/othervm -ea ParseCanonicalPath
  */

A better alternative might be to throw a RuntimeException instead of using 
assert.

Thanks,

Brian

On Feb 14, 2018, at 2:36 PM, yumin qi  wrote:

>   Updated again in same link.
>   I could not run jtreg so please have a look if the options are good for @run

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

[jesper . wilhelmsson]

Looks good!
/Jesper

> On 14 Feb 2018, at 22:45, 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
> 
> Thanks,
> Sangheon
> 

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

[sangheon.kim]

Hi David and Jesper,

Thank you for the review.

Sangheon


On 02/14/2018 02:53 PM, jesper.wilhelms...@oracle.com wrote:

Looks good!
/Jesper


On 14 Feb 2018, at 22:45, 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

Thanks,
Sangheon

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

[mandy chung]

+1

Mandy

On 2/14/18 1:45 PM, 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

Thanks,
Sangheon

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

[yumin qi]

Brian,

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

Thanks
Yumin

On Wed, Feb 14, 2018 at 11:17 AM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> Hello,
>
> In the test you have
>
> /* @test requires (os.family == "linux") | (os.family == "mac") | (os.family 
> == "solaris") | (os.family == "aix")
>@bug 8194154
>@summary Test parsing path with double slashes on unix like platforms.
>  */
>
> but I think you need to have the @requires annotation on a separate line
> for example as:
>
> /* @test
>  * @requires (os.family == "linux") | (os.family == "mac”)
>  * | (os.family == "solaris") | (os.family == "aix")
>  * @bug 8194154
>  * @summary Test parsing path with double slashes on unix like platforms.
>  */
>
> Thanks,
>
> Brian
>
> On Feb 14, 2018, at 10:48 AM, yumin qi  wrote:
>
>  Thanks. Updated on same link
>   http://cr.openjdk.java.net/~minqi/8194154/webrev1/
>  as your recommendation.
>
>
>

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

[sangheon.kim]

Hi Mandy,

Thank you for the review!

Sangheon


On 02/14/2018 03:45 PM, mandy chung wrote:

+1

Mandy

On 2/14/18 1:45 PM, 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

Thanks,
Sangheon

Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

[Stuart Marks]

Hi Joe,

Overall, looks good.

Are there any tests of AbstractStringBuilder.compareTo() that exercise 
comparisons of the Latin1 vs UTF16 coders?


A couple people have raised the issue of the SB's implementing Comparable but 
not overriding equals(). This is unusual but well-defined. I do think it 
deserves the "inconsistent with equals" treatment. Something like the following 
should be added to both SB's:


==
@apiNote
{@code StringBuilder} implements {@code Comparable} but does not override {@link 
Object#equals equals}. Thus, the natural ordering of {@code StringBuilder} is 
inconsistent with equals. Care should be exercised if {@code StringBuilder} 
objects are used as keys in a {@code SortedMap} or elements in a {@code 
SortedSet}. See {@link Comparable}, {@link java.util.SortedMap SortedMap}, or 
{@link java.util.SortedSet SortedSet} for more information.

==

Respectively for StringBuffer. (Adjust markup to taste.)

Thanks,

s'marks




On 2/8/18 4:47 PM, Joe Wang wrote:

Hi all,

The CSR for the enhancement is now approved. Thanks Joe!

The webrev has been updated accordingly. Please let me know if you have any 
further comment on the implementation.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 2/2/2018 12:46 PM, Joe Wang wrote:

Thanks Jason. Will update that accordingly.

Best,
Joe

On 2/2/2018 11:22 AM, Jason Mehrens wrote:

Joe,

The identity check in CS.compare makes sense.  However, it won't be null 
hostile if we call CS.compare(null, null) and that doesn't seem right.

Usually when writing comparator classes I end up with:
===
if (Objects.requireNonNull(o1) == Objects.requireNonNull(o2)) {
 return 0;
}
===

Jason

From: core-libs-dev  on behalf of Joe 
Wang 

Sent: Friday, February 2, 2018 1:01 PM
To: core-libs-dev
Subject: Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, 
StringBuilder, and StringBuffer


Hi,

Thanks all for comments and suggestions. I've updated the webrev. Please
review.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 1/31/2018 9:31 PM, Joe Wang wrote:

Hi Tagir,

Thanks for the comment. I will consider adding that to the javadoc
emphasizing that the comparison is performed from 0 to length() - 1 of
the two sequences.

Best,
Joe

On 1/29/18, 8:07 PM, Tagir Valeev wrote:

Hello!

An AbstractStringBuilder#compareTo implementation is wrong. You cannot
simply compare the whole byte array. Here's the test-case:

public class Test {
    public static void main(String[] args) {
  StringBuilder sb1 = new StringBuilder("test1");
  StringBuilder sb2 = new StringBuilder("test2");
  sb1.setLength(4);
  sb2.setLength(4);
  System.out.println(sb1.compareTo(sb2));
System.out.println(sb1.toString().compareTo(sb2.toString()));
    }
}

We truncated the stringbuilders making their content equal, so
sb1.toString().compareTo(sb2.toString()) is 0, but compareTo compares
the original content (before the truncation) as truncation, of course,
does not zero the truncated bytes, neither does it reallocate the
array (unless explicitly asked via trimToSize).

With best regards,
Tagir Valeev.


On Fri, Jan 26, 2018 at 10:00 AM, Joe Wang
wrote:

Hi,

Adding methods for comparing CharSequence, StringBuilder, and
StringBuffer.

The Comparable implementations for StringBuilder/Buffer are similar
to that
of String, allowing comparison operations between two
StringBuilders/Buffers, e.g.
aStringBuilder.compareTo(anotherStringBuilder).
For CharSequence however, refer to the comments in JIRA, a static
method
'compare' is added instead of implementing the Comparable interface.
This
'compare' method may take CharSequence implementations such as String,
StringBuilder and StringBuffer, making it possible to perform
comparison
among them. The previous example for example is equivalent to
CharSequence.compare(aStringBuilder, anotherStringBuilder).

Tests for java.base have been independent from each other. The new
tests are
therefore created to have no dependency on each other or sharing any
code.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

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

[David Holmes]

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:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-October/049597.html

"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
-


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://bugs.openjdk.java.net/browse/JDK-8190187

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

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

[Martin Buchholz]

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.

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

[Peter Levart]

Hi Martin,

On 02/15/2018 06:47 AM, Martin Buchholz wrote:

Embarrassed by my failure to take runFinalizersOnExit into account, I tried
to redo the patch to be actually correct.


I think that your patch was correct the first time too, but in view of 
future removing the need for "pollFirst" from unfinalized list, the 
revised patch separates this logic entirely into the to-be-removed 
method so it is preferable.



  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.



...and the check for "already been finalized" needs to be performed only 
in deregisterAndRunFinalizer as you did in new patch. Once 
runAllFinalizers is removed, this check and marking code could be 
removed too.


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.


Regards, Peter

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

[Xueming Shen]

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.

Thanks,
Sherman