Re: RFR 8196668: revisit test SunPackageAccess and GrantedSunPackageAccess

[Chris Yin]

Thank you, Mandy. I just fixed it follow your comments and get it pushed.

Regards,
Chris

> On 29 Mar 2018, at 12:48 PM, mandy chung  > wrote:
> 
> 
> 
> On 3/29/18 9:59 AM, Chris Yin wrote:
>> Please review the change to merge 2 package access tests and move to 
>> OpenJDK, thanks
>> 
>> bug: https://bugs.openjdk.java.net/browse/JDK-8196668 
>>  
>>  
>> 
>> webrev: http://cr.openjdk.java.net/~xyin/8196668/webrev.00/ 
>>  
>>  
>> 
>> 
> 
> Looks okay.  Minor comments:
>   88 throw new RuntimeException("Unexpected 
> AccessControlException",
>   89 ace);
> 
>   92 throw new RuntimeException("Test failed with unexpected 
> exception",
>   93 ex);
> 
> Nit: each throw statement can be merged in 1 line.
> 
> test/jdk/java/lang/SecurityManager/empty.policy
>can you add a comment saying this is an empty policy.
> 
> No need to generate a new webrev.  You can fix it before you push.
> 
> Mandy

Re: RFR 8196668: revisit test SunPackageAccess and GrantedSunPackageAccess

[mandy chung]

On 3/29/18 9:59 AM, Chris Yin wrote:

Please review the change to merge 2 package access tests and move to OpenJDK, 
thanks

bug: https://bugs.openjdk.java.net/browse/JDK-8196668 

webrev: http://cr.openjdk.java.net/~xyin/8196668/webrev.00/ 




Looks okay.  Minor comments:

  88 throw new RuntimeException("Unexpected 
AccessControlException",
  89 ace);

  92 throw new RuntimeException("Test failed with unexpected 
exception",
  93 ex);

Nit: each throw statement can be merged in 1 line.

test/jdk/java/lang/SecurityManager/empty.policy
   can you add a comment saying this is an empty policy.

No need to generate a new webrev.  You can fix it before you push.

Mandy

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

[Xueming Shen]

On 3/28/18, 6:14 AM, David Lloyd wrote:



The current wording (which pre-dates your changes of course) reads more like
API advice. I think it's a bit confusing too as it doesn't define what a
"flush marker" is. I think we will need to re-word that sentence to make it
clearer.

OK.  I am at a loss for a better way to explain it though; any suggestions?

I've implemented all the other changes except for this one.  Latest
version is attached, online version is here [1].




"flush marker" was copy/pasted from the original zlib.h api doc when I 
added the
support for different flush options. It might be better to put it is a 
apiNote. A
"flush marker" in this case is a 5-byte empty block, which will be 
output if the
deflater does not have enough space to output the bytes and it is in 
full_flush or
sync_flush mode. I think you can leave it as is for now and I will try 
to see if I

can move it around or add some understandable wording, if I can.

-Sherman

RFR 8196668: revisit test SunPackageAccess and GrantedSunPackageAccess

[Chris Yin]

Please review the change to merge 2 package access tests and move to OpenJDK, 
thanks

bug: https://bugs.openjdk.java.net/browse/JDK-8196668 

webrev: http://cr.openjdk.java.net/~xyin/8196668/webrev.00/ 


Regards,
Chris

Re: RFR: Here are some Thread cleanup patches

[mandy chung]

On 3/29/18 1:54 AM, Martin Buchholz wrote:



On Wed, Mar 28, 2018 at 4:30 AM, mandy chung > wrote:



I was wondering how Martin made the decision to mark it with
@Stable. In addition to the above, the ID is mostly used in
tracing.  I'd like to see performance number for adding @Stable.


OK, I had "no good reason" for using @Stable here, except that all 
these core library classes are fundamentally performance sensitive.  
@Stable has been removed.



+1


8200123: Replace Thread.init with telescoping constructor
http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-init/ 


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


Looks good.

Mandy

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

On 2018-03-28 23:53, Martin Buchholz wrote:

I can't find any documentation for what JNIEXPORT and friends actually do.
People including myself have been cargo-culting JNIEXPORT and JNICALL 
for decades.

Why aren't they in the JNI spec?
That surprises me. I'm quite certain that javah (or rather, java -h 
nowadays) generate header files with JNIEXPORT and JNICALL.


As you can see in the jni.h and jni_md.h files, JNIEXPORT equals 
__attribute__((visibility("default"))) for compilers that support it 
(gcc and friends), and __declspec(dllexport) for Windows. This means, 
that the symbol should be exported. (And it's ignored if you use 
mapfiles aka linker scripts.)


As for JNICALL, it's empty on most compilers, but evaluates to __stdcall 
on Windows. This defines the calling convention to use. This is required 
for JNI calls from Java. (Ask the JVM team why.) While it's not 
technically required for calling from one dll to another, it's good 
practice to use it all time to be consistent. In any way, it doesn't 
hurt us.




---

It's fishy that the attribute externally_visible (which seems very 
interesting!) is ARM specific.


  #ifdef ARM
    #define JNIEXPORT 
 __attribute__((externally_visible,visibility("default")))
    #define JNIIMPORT 
 __attribute__((externally_visible,visibility("default")))


Yeah, this is broken on so many levels. :-(

The ARM here goes back to the old Oracle proprietary arm32 port. This 
used lto, link time optimization, to get an absolutely minimal runtime, 
at expense of a extremely long built time. (I think linking libjvm took 
like 20 minutes.) But when using lto, you also need to decorate your 
functions with the externally_visible attribute. So this was added to 
get hotspot to export the proper symbols (since they, too, used the 
jni.h file).


So, in short, we should:
1) have used a special, local jni.h file for the proprietary arm port, 
and/or
2) added the externally_visible attribute not based on platform, but on 
the existence of lto.


At this point in time, we're not building the old 32-bit arm port, and I 
doubt anyone does. And even if so, we could probably remove the lto 
part, and thus remove this from jni_md.h. If you want, please file a bug.


/Magnus


  #else
    #define JNIEXPORT  __attribute__((visibility("default")))
    #define JNIIMPORT  __attribute__((visibility("default")))
  #endif

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Martin Buchholz]

I can't find any documentation for what JNIEXPORT and friends actually do.
People including myself have been cargo-culting JNIEXPORT and JNICALL for
decades.
Why aren't they in the JNI spec?

---

It's fishy that the attribute externally_visible (which seems very
interesting!) is ARM specific.

  #ifdef ARM
#define JNIEXPORT
 __attribute__((externally_visible,visibility("default")))
#define JNIIMPORT
 __attribute__((externally_visible,visibility("default")))
  #else
#define JNIEXPORT __attribute__((visibility("default")))
#define JNIIMPORT __attribute__((visibility("default")))
  #endif

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Erik Joelsson]

Build changes still look good to me.

/Erik


On 2018-03-28 03:31, Magnus Ihse Bursie wrote:

On 2018-03-28 01:52, Weijun Wang wrote:


On Mar 24, 2018, at 6:03 AM, Magnus Ihse Bursie 
 wrote:


https://bugs.openjdk.java.net/browse/JDK-8200193 -- for 
jdk.security.auth
There is only one function to export and it already has JNIEXPORT, so 
you can just remove the new $(LIBJAAS_CFLAGS) [1].

Ok, thanks Max!

Are you going to update your webrev?
Here is a new webrev. It includes your recommended change in 
Lib-jdk.security.auth.gmk.


It is also updated to keep track of changes in shared native libraries 
that has happend in the mainline since my first webrev. Most notably 
is the addition of libjsig. For now, I have just added the JNIEXPORT 
markers for the platforms that need it. Hopefully we can unify libjsig 
across all platforms, but that seems to be more complicated than I 
thought, so that'll have to wait.


I have also recieved word from Phil Race that there were no testing 
issues for client, so he's happy as well.


Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.03


/Magnus



Thanks
Max

[1] 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01/make/lib/Lib-jdk.security.auth.gmk.sdiff.html

Re: RFR 8197595: Serialization javadoc should link to security best practices

[Lance Andersen]

Hi Roger,

Looks good to go to me!

Best
Lance
> On Mar 28, 2018, at 1:27 PM, Roger Riggs  wrote:
> 
> Hi,
> 
> Updated with editorial suggestions.
> 
> webrev:
> http://cr.openjdk.java.net/~rriggs/webrev-serialwarn-8197595/index.html 
> 
> 
> javadoc:
> http://cr.openjdk.java.net/~rriggs/serialwarn/api/java.base/java/io/package-summary.html
>  
> 
> 
> Thanks for the reviews, Roger
> 
> On 3/23/2018 12:57 PM, Lance Andersen wrote:
>> Looks good to me also Roger with Sean’s suggestions :-)
>> 
>>> On Mar 23, 2018, at 10:12 AM, Roger Riggs >> > wrote:
>>> 
>>> Please review adding a warning and a link to the Secure Coding Guidelines
>>> and the new Serial Filter guide[2] included in the JDK 10 docs.
>>> The warnings are added to Serializable, ObjectInputStream, 
>>> ObjectInputFilter and
>>> the java.io  package summary.
>>> 
>>> webrev:
>>> http://cr.openjdk.java.net/~rriggs/webrev-serialwarn-8197595/index.html 
>>> 
>>> 
>>> javadoc:
>>> http://cr.openjdk.java.net/~rriggs/serialwarn/api/java.base/java/io/package-summary.html
>>>  
>>> 
>>> 
>>> Thanks, Roger
>>> 
>>> [2] 
>>> https://docs.oracle.com/javase/10/core/serialization-filtering1.htm#JSCOR-GUID-3ECB288D-E5BD-4412-892F-E9BB11D4C98A
>>>  
>>> 
>>> 
>>> 
>> 
>>  
>> 
>>   
>> 
>>  Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> lance.ander...@oracle.com 
>> 
>> 
>> 
> 

 
  

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

Re: RFR: Here are some Thread cleanup patches

[Martin Buchholz]

On Wed, Mar 28, 2018 at 4:30 AM, mandy chung  wrote:

>
> I was wondering how Martin made the decision to mark it with @Stable. In
> addition to the above, the ID is mostly used in tracing.  I'd like to see
> performance number for adding @Stable.
>

OK, I had "no good reason" for using @Stable here, except that all these
core library classes are fundamentally performance sensitive.  @Stable has
been removed.

8200123: Replace Thread.init with telescoping constructor
http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-init/
https://bugs.openjdk.java.net/browse/JDK-8200123

Re: RFR 8197595: Serialization javadoc should link to security best practices

[Roger Riggs]

Hi,

Updated with editorial suggestions.

webrev:
http://cr.openjdk.java.net/~rriggs/webrev-serialwarn-8197595/index.html 



javadoc:
http://cr.openjdk.java.net/~rriggs/serialwarn/api/java.base/java/io/package-summary.html

Thanks for the reviews, Roger

On 3/23/2018 12:57 PM, Lance Andersen wrote:

Looks good to me also Roger with Sean’s suggestions :-)

On Mar 23, 2018, at 10:12 AM, Roger Riggs > wrote:


Please review adding a warning and a link to the Secure Coding Guidelines
and the new Serial Filter guide[2] included in the JDK 10 docs.
The warnings are added to Serializable, ObjectInputStream, 
ObjectInputFilter and

the java.io  package summary.

webrev:
http://cr.openjdk.java.net/~rriggs/webrev-serialwarn-8197595/index.html 



javadoc:
http://cr.openjdk.java.net/~rriggs/serialwarn/api/java.base/java/io/package-summary.html

Thanks, Roger

[2] 
https://docs.oracle.com/javase/10/core/serialization-filtering1.htm#JSCOR-GUID-3ECB288D-E5BD-4412-892F-E9BB11D4C98A







Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

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

Re: RFR: 8200238: Reduce number of exceptions created when calling MemberName$Factory::resolveOrNull

[Karen Kinnear]

Claes,

Thank you for this fix. Glad it makes a startup difference. Agree with Lois.

thanks,
Karen

p.s. haven’t had a chance to trace the logic of your explanation below in a 
debugger - I don’t
quite follow it. But the fix makes sense.

> On Mar 27, 2018, at 11:57 AM, Lois Foltan  wrote:
> 
> On 3/27/2018 2:49 AM, Claes Redestad wrote:
> 
>> 
>> 
>> On 2018-03-26 17:51, Claes Redestad wrote:
>>> Karen,
>>> 
>>> On 2018-03-26 17:15, Karen Kinnear wrote:
 Claes,
 
 Discussed with Lois. We think that it would make more sense to pass the 
 new argument into MethodHandles::resolve_MemberName and at all three 
 places that we currently CHECK_PENDING_EXCEPTION/return null there
 - if speculative flag is set - CLEAR_PENDING_EXCEPTION before you 
 return null
 - and yes - do this for all three cases, not just the METHOD case
>>> 
>>> ok.
>> 
>> New webrev:
>> 
>> http://cr.openjdk.java.net/~redestad/8200238/open.01/
> 
> Hi Claes,
> Looks good.  One minor comment.
> 
> hotspot/share/prims/methodHandles.cpp:
> - line #1237.  Consider putting some explanation in the assert statement 
> instead of a blank string.  Something like "speculative resolve mode has 
> encountered an unexpected pending exception"
> 
> I don't need to see another webrev.
> 
> Thanks,
> Lois
> 
>> 
>> Thanks!
>> 
>> /Claes

Re: [JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

[David Lloyd]

On Tue, Mar 27, 2018 at 11:59 AM, David Lloyd  wrote:
> On Tue, Mar 27, 2018 at 10:10 AM, Alan Bateman  
> wrote:
>> On 27/03/2018 00:09, David Lloyd wrote:
>>> I was going for some kind of consistency with the array variants (that
>>> is, name the parameter what it is), though they simply use "b" for
>>> their parameter name so that interpretation might be a stretch.
>>> Should I update them all?
>>
>> I think this would be good, if you don't mind doing it.
>
> OK.
>
>>> I'm not 100% familiar with the new JavaDoc categories (ok I'm 0%
>>> familiar), but the JEP for these says "This category consists of
>>> commentary, rationale, or examples pertaining to the API.".  But this
>>> feels more like specification to me since it is "specifications that
>>> apply to all valid implementations, including preconditions,
>>> postconditions, etc.".  Which is to say, if you don't provide enough
>>> remaining space in the output buffer, you will have incorrect
>>> operation as a result.  WDYT?
>>
>> The current wording (which pre-dates your changes of course) reads more like
>> API advice. I think it's a bit confusing too as it doesn't define what a
>> "flush marker" is. I think we will need to re-word that sentence to make it
>> clearer.
>
> OK.  I am at a loss for a better way to explain it though; any suggestions?

I've implemented all the other changes except for this one.  Latest
version is attached, online version is here [1].

[1] https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v18


-- 
- DML
commit 55e4df2ab3c7ccb68426bc90f3cc5d2d1a3de96e
Author: David M. Lloyd 
Date:   Fri Feb 16 11:00:10 2018 -0600

[JDK-6341887] Update Inflater/Deflater to handle ByteBuffer

diff --git a/make/mapfiles/libzip/mapfile-vers 
b/make/mapfiles/libzip/mapfile-vers
index d711d8e17f4..11ccc2d6ecb 100644
--- a/make/mapfiles/libzip/mapfile-vers
+++ b/make/mapfiles/libzip/mapfile-vers
@@ -33,20 +33,28 @@ SUNWprivate_1.1 {
Java_java_util_zip_CRC32_update;
Java_java_util_zip_CRC32_updateBytes0;
Java_java_util_zip_CRC32_updateByteBuffer0;
-   Java_java_util_zip_Deflater_deflateBytes;
+   Java_java_util_zip_Deflater_deflateBytesBytes;
+   Java_java_util_zip_Deflater_deflateBytesBuffer;
+   Java_java_util_zip_Deflater_deflateBufferBytes;
+   Java_java_util_zip_Deflater_deflateBufferBuffer;
Java_java_util_zip_Deflater_end;
Java_java_util_zip_Deflater_getAdler;
Java_java_util_zip_Deflater_init;
Java_java_util_zip_Deflater_initIDs;
Java_java_util_zip_Deflater_reset;
Java_java_util_zip_Deflater_setDictionary;
+   Java_java_util_zip_Deflater_setDictionaryBuffer;
Java_java_util_zip_Inflater_end;
Java_java_util_zip_Inflater_getAdler;
-   Java_java_util_zip_Inflater_inflateBytes;
+   Java_java_util_zip_Inflater_inflateBytesBytes;
+   Java_java_util_zip_Inflater_inflateBytesBuffer;
+   Java_java_util_zip_Inflater_inflateBufferBytes;
+   Java_java_util_zip_Inflater_inflateBufferBuffer;
Java_java_util_zip_Inflater_init;
Java_java_util_zip_Inflater_initIDs;
Java_java_util_zip_Inflater_reset;
Java_java_util_zip_Inflater_setDictionary;
+   Java_java_util_zip_Inflater_setDictionaryBuffer;
ZIP_Close;
ZIP_CRC32;
ZIP_FreeEntry;
diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java 
b/src/java.base/share/classes/java/util/zip/Deflater.java
index c75dd4a33f0..85e3debb393 100644
--- a/src/java.base/share/classes/java/util/zip/Deflater.java
+++ b/src/java.base/share/classes/java/util/zip/Deflater.java
@@ -26,7 +26,13 @@
 package java.util.zip;
 
 import java.lang.ref.Cleaner.Cleanable;
+import java.lang.ref.Reference;
+import java.nio.ByteBuffer;
+import java.nio.ReadOnlyBufferException;
+import java.util.Objects;
+
 import jdk.internal.ref.CleanerFactory;
+import sun.nio.ch.DirectBuffer;
 
 /**
  * This class provides support for general purpose compression using the
@@ -92,8 +98,9 @@ import jdk.internal.ref.CleanerFactory;
 public class Deflater {
 
 private final DeflaterZStreamRef zsRef;
-private byte[] buf = new byte[0];
-private int off, len;
+private ByteBuffer input = ZipUtils.defaultBuf;
+private byte[] inputArray;
+private int inputPos, inputLim;
 private int level, strategy;
 private boolean setParams;
 private boolean finish, finished;
@@ -170,9 +177,14 @@ public class Deflater {
  */
 public static final int FULL_FLUSH = 3;
 
+/**
+ * Flush mode to use at the end of output.  Can only be provided by the
+ * user by way of {@link #finish()}.
+ */
+private static 

Re: RFR: Here are some Thread cleanup patches

[mandy chung]

On 3/28/18 6:34 PM, Peter Levart wrote:



On 03/28/2018 11:32 AM, mandy chung wrote:

On 28/03/2018 4:28 AM, Martin Buchholz wrote:
> The usual story when I change Thread.java is that I'm missing 
something and

> I end up reverting, so I was extra careful this time.
>
> 8200122: Remove unused field Thread.threadQ
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-threadQ/
> https://bugs.openjdk.java.net/browse/JDK-8200122

Looks fine.

> 8200123: Replace Thread.init with telescoping constructor
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-init/
> https://bugs.openjdk.java.net/browse/JDK-8200123

Looks good too.

Did you see any performance difference with and without @Stable?

Mandy


I doubt there would be any. @Stable is only effective when the 
reference to an object containing the annotated field can be constant 
folded by JIT. And Thread objects are typically not assigned to static 
final field(s) or (by transitivity) to @Stable fields. 
Thread.currentThread() could be an exception, but it's not a constant. 
It's a thread-local constant, so a possible optimization would be for 
JIT to cache Thread's @Stable fields in a thread-local storage, but I 
doubt it is programmed to do so.




I was wondering how Martin made the decision to mark it with @Stable. In 
addition to the above, the ID is mostly used in tracing.  I'd like to 
see performance number for adding @Stable.


Mandy


VM experts, please advise!

Regards, Peter

Re: RFR: Here are some Thread cleanup patches

[Peter Levart]

On 03/28/2018 11:32 AM, mandy chung wrote:

On 28/03/2018 4:28 AM, Martin Buchholz wrote:
> The usual story when I change Thread.java is that I'm missing 
something and

> I end up reverting, so I was extra careful this time.
>
> 8200122: Remove unused field Thread.threadQ
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-threadQ/
> https://bugs.openjdk.java.net/browse/JDK-8200122

Looks fine.

> 8200123: Replace Thread.init with telescoping constructor
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-init/
> https://bugs.openjdk.java.net/browse/JDK-8200123

Looks good too.

Did you see any performance difference with and without @Stable?

Mandy


I doubt there would be any. @Stable is only effective when the reference 
to an object containing the annotated field can be constant folded by 
JIT. And Thread objects are typically not assigned to static final 
field(s) or (by transitivity) to @Stable fields. Thread.currentThread() 
could be an exception, but it's not a constant. It's a thread-local 
constant, so a possible optimization would be for JIT to cache Thread's 
@Stable fields in a thread-local storage, but I doubt it is programmed 
to do so.


VM experts, please advise!

Regards, Peter

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

On 2018-03-28 01:52, Weijun Wang wrote:



On Mar 24, 2018, at 6:03 AM, Magnus Ihse Bursie  
wrote:

https://bugs.openjdk.java.net/browse/JDK-8200193 -- for jdk.security.auth

There is only one function to export and it already has JNIEXPORT, so you can 
just remove the new $(LIBJAAS_CFLAGS) [1].

Ok, thanks Max!

Are you going to update your webrev?
Here is a new webrev. It includes your recommended change in 
Lib-jdk.security.auth.gmk.


It is also updated to keep track of changes in shared native libraries 
that has happend in the mainline since my first webrev. Most notably is 
the addition of libjsig. For now, I have just added the JNIEXPORT 
markers for the platforms that need it. Hopefully we can unify libjsig 
across all platforms, but that seems to be more complicated than I 
thought, so that'll have to wait.


I have also recieved word from Phil Race that there were no testing 
issues for client, so he's happy as well.


Updated webrev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.03


/Magnus



Thanks
Max

[1] 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01/make/lib/Lib-jdk.security.auth.gmk.sdiff.html

Re: RFR: Here are some Thread cleanup patches

[mandy chung]

On 3/28/18 2:37 PM, Peter Levart wrote:

Hi,

As far as I think (but please prove me wrong), Stable class is not 
loaded until someone explicitly asks for it in Java code (referring to 
Stable type in bytecode like with Stable.class literal or parsing 
annotations of a field that contains such annotation triggered by Java 
API for annotations).


The VM code that parses some annotations like @Stable, 
@CallerSensitive, @ForceInline, etc... works by matching the Symbol 
names of annotation classes and doesn't need the to load the 
annotation class for that. See parse_annotations in 
classFileParser.cpp...




This is what I understand too.  VM parses the annotation attributes 
directly and no need to load any annotation class.


Mandy

Re: RFR: Here are some Thread cleanup patches

[mandy chung]

On 28/03/2018 4:28 AM, Martin Buchholz wrote:
> The usual story when I change Thread.java is that I'm missing 
something and

> I end up reverting, so I was extra careful this time.
>
> 8200122: Remove unused field Thread.threadQ
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-threadQ/
> https://bugs.openjdk.java.net/browse/JDK-8200122

Looks fine.

> 8200123: Replace Thread.init with telescoping constructor
> http://cr.openjdk.java.net/~martin/webrevs/jdk/Thread-init/
> https://bugs.openjdk.java.net/browse/JDK-8200123

Looks good too.

Did you see any performance difference with and without @Stable?

Mandy

Re: [11] RFR: 8191410 : Unicode 10.0.0

[Ivan Gerasimov]

Thanks Rachna!


On 3/13/18 2:28 AM, Rachna Goel wrote:


Hi Roger, Ivan,

There is no change in algorithm as such but there has been mainly 
stability improvements, defects fixed, performance enhancements. e.g 
Updated header check, version and loading for latest binary files, 
Simple case mappings will be more efficient as unnecessary checking 
has been removed.

complete list of changes can be found at:

http://bugs.icu-project.org/trac/query?status=closed=fixed=fixedbyotherticket=60.1=60.2=project=999=id=summary=resolution=milestone=status=owner=type=priority=project=weeks=priority

Also, I have incorporated  suggestions given from Ivan.
Kindly have a look at updated webrev:

http://cr.openjdk.java.net/~rgoel/JDK-8191410/webrev.01/



Please update the comment at the line 675 in Character.java:
* 638  - the expected number of entities

Please also update the test 
test/jdk/java/lang/Character/UnicodeBlock/OptimalMapSize.java:

the expected capacity 510 needs to be replaced with 638

With kind regards,
Ivan


Thanks,
Rachna


On 3/8/18 9:47 PM, Roger Riggs wrote:

Hi Rachna,

sun/text/normalizer/NormalizerImpl.java:

Is there a higher level description of the algorithmic changes?

102-105:   These look like accidental changes: the space should not 
be removed and the trailing "{" doesn't make sense in a comment.


Regards, Roger


On 3/8/2018 6:56 AM, Rachna Goel wrote:

Hi,

Please review the proposed changes for JDK-819410.

Bug : https://bugs.openjdk.java.net/browse/JDK-8191410

proposed changeset is located at :

http://cr.openjdk.java.net/~rgoel/JDK-8191410/webrev/

This serves as the implementation for JEP 327.





--
Thanks,
Rachna


--
With kind regards,
Ivan Gerasimov

Re: RFR: Here are some Thread cleanup patches

[Peter Levart]

Hi,

As far as I think (but please prove me wrong), Stable class is not 
loaded until someone explicitly asks for it in Java code (referring to 
Stable type in bytecode like with Stable.class literal or parsing 
annotations of a field that contains such annotation triggered by Java 
API for annotations).


The VM code that parses some annotations like @Stable, @CallerSensitive, 
@ForceInline, etc... works by matching the Symbol names of annotation 
classes and doesn't need the to load the annotation class for that. See 
parse_annotations in classFileParser.cpp...


Regards, Peter


On 03/28/18 04:21, David Holmes wrote:

On 28/03/2018 12:07 PM, Martin Buchholz wrote:
java -Xlog:class+init=trace -version |& grep -Ew 
'annotation|lang.Thread|Stable'
[0.019s][info][class,init] 10 Initializing 'java/lang/Thread' 
(0x0007c0006400)


Thanks.

Intuitively, class+init should be a subset of class+load, and the 
experiment above supports that.


The initialization order can be quite different to the load order.

David

On Tue, Mar 27, 2018 at 6:59 PM, David Holmes 
> wrote:


    On 28/03/2018 11:50 AM, Martin Buchholz wrote:



    On Tue, Mar 27, 2018 at 6:24 PM, Martin Buchholz
    
    >> 
wrote:


       At least the VM doesn't have to run any risky java code


    ??  Why is Martin so sure ??
    Let's check:

    java -Xlog:class+load=trace -version |& grep -Ew
    'annotation|lang.Thread'
    [0.010s][info ][class,load] java.lang.Thread source: 
jrt:/java.base

    [0.010s][info ][class,load]
    java.lang.Thread$UncaughtExceptionHandler source: jrt:/java.base
    [0.012s][info ][class,load] java.lang.annotation.Annotation
    source: jrt:/java.base

    So Stable does __not__ have to be class-loaded when Thread is
    class-loaded.


    Can you check with class+initialization please.

    Thanks,
    David