Re: RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

[Xueming Shen]

On 7/6/18, 5:43 PM, Martin Buchholz wrote:
I would use different timestamps for the 4 possible times used in this 
test, if only to make it clearer which value comes from where.


webrev has been updated accordingly.



+// ze.setLastModifiedTime(now);
+ze.setTime(now.toMillis());

setTime only sets the DOS time?  Which only has a granularity of 2 
seconds?  If so, how do you get back the same value you put in if the 
current second is odd?  Or am I misunderstanding the test?




no misunderstanding, good catch. The test does fail when hits the odd 
second.

added a special "check" version for the 2 second granularity set/getTime().

Thanks,
Sherman

Re: RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

[Martin Buchholz]

I would use different timestamps for the 4 possible times used in this
test, if only to make it clearer which value comes from where.

+// ze.setLastModifiedTime(now);+
ze.setTime(now.toMillis());


setTime only sets the DOS time?  Which only has a granularity of 2
seconds?  If so, how do you get back the same value you put in if the
current second is odd?  Or am I misunderstanding the test?


On Fri, Jul 6, 2018 at 4:46 PM, Xueming Shen 
wrote:

> Hi
>
> Please help review the changeset for JDK-8206389
>
> issue: https://bugs.openjdk.java.net/browse/JDK-8206389
> webrev: http://cr.openjdk.java.net/~sherman/8206389/webrev
>
> Cause: ZipOutputStream.writeCEN().writeCEN() incorrectly calculate the
> length
> of bytes needed for the  "unix timestamps" when the "last modified time" is
> NOT set. The info-zip spec specifies that
>
>   The central-header extra field contains the modification time
> only,
>   or no timestamp at all.  TSize is used to flag its presence or
>   absence.  But note:
>
> So in this case, when "creation time", "last access time" are set but the
> "last modified time" is not. only the "tag", "size" and "flag" should be
> output to
> the extra field, no real "mtime" timestamp, so the total size of the bytes
> needed
> is 5, not 9.
>
> Thanks,
> Sherman
>
>

RFR JDK-8206389: JarEntry.setCreation/LastAccessTime without setLastModifiedTime causes Invalid CEN header

[Xueming Shen]

Hi

Please help review the changeset for JDK-8206389

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

Cause: ZipOutputStream.writeCEN().writeCEN() incorrectly calculate the 
length

of bytes needed for the  "unix timestamps" when the "last modified time" is
NOT set. The info-zip spec specifies that

  The central-header extra field contains the modification time 
only,

  or no timestamp at all.  TSize is used to flag its presence or
  absence.  But note:

So in this case, when "creation time", "last access time" are set but the
"last modified time" is not. only the "tag", "size" and "flag" should be 
output to
the extra field, no real "mtime" timestamp, so the total size of the 
bytes needed

is 5, not 9.

Thanks,
Sherman

Re: Prototype of jpackager in jdk/sandbox [was: Draft JEP proposal: JDK-8200758: Packaging Tool]

[Remi Forax]

I've just taking a look at the patch,
i don't see how this can be integrated soon, the code is consistently 
inconsistent as one of my colleague would say, even the coding conventions are 
not respected.
i believe that's it's because the code have been written first in Java 6 an 
without refactoring was moved to use Java 7, 8, 9, 10 and 11.

The I/O code still using java.io.File for some parts, no try-with-resources so 
most of the try/finally are not safe,
a lot of code like new BufferedWriter(new FileWriter(file)) instead of 
Files.newBufferedWriter, etc. The code should use the package java.nio.file, 
and not the old java.io,
most of the code try to manage the exception right were they appear instead of 
propagating them so there are too many try/catch,
a lot of catch are ignored which is a code smell, some codes use the internal 
logger (jdk.packager.internal.Log), but a lot of codes doesn't,
for the collection code, there is a lot of copy of data small structures (which 
suggest that published collections are not immutable),
there are dubious @SuppressWarnings("unchecked"), some or them are due to the 
fact that the code use Class as a type token instead of using lambdas,
Stream are not used when they should (to avoid multiple copy between data 
structures) and streams that need to be closed are not (the result of 
Files.list by example),
there are usual "don't do that in Java" like a loop using an integer index to 
traverse a List without knowing if it's a random access list or not,
there is a lot of nullchecks instead of using Optional,
a lot of code initialize local variables to null which is a code smell (and a 
side effect of having a lot of try/catch but not only),
constructors should not do work, just initialization, use static factory method 
instead (so you will not have to debug half constructed objects),
the code uses BigIntegers to parse a bundle version, just in case,
the code uses an AtomicReference as a box that a lambda can mutate, instead of 
wrapping the exception into a runtime and unwrapping it at call site,
The code of jdk.packager.internal.IOUtils should be updated to use methods of 
the JDK 11 and methods like readFully should be replaced by the JDK's one.
listOfPathToString and setOfStringToString are just WTF, like in 
getRedistributableModules(), where the variable stream is an Optional,
A class like Platform should be used everywhere to do platform specific stuff, 
a lot of code still use String matching (the version parsing should use 
System.Version).
All the argument parsing should be delegated to JOpt (the one integrated with 
the JDK), so it will be consistent with the other JDK tools,
There is a UnsupportedPlatformException but a Platform can be UNKOWN ??

I will stop here, my point is that there is a lot of cleaning that should 
appear before the code is integrated into the JDK and given the size of the 
code, i wonder if it's not better to start an OpenJDK project for it and 
iterate on the code before trying to include it in the JDK. For me, the code is 
too big to use the jdk/sandbox.

regards,
Rémi

- Mail original -
> De: "Kevin Rushforth" 
> À: "core-libs-dev" 
> Cc: "Alexey Semenyuk" , "Andy Herrick" 
> 
> Envoyé: Vendredi 6 Juillet 2018 22:14:29
> Objet: Prototype of jpackager in jdk/sandbox [was: Draft JEP proposal: 
> JDK-8200758: Packaging Tool]

> An initial prototype of the jpackager tool has been pushed to a new
> 'JDK-8200758-branch' branch in the JDK sandbox [1]. If anyone is
> interested in taking a look, you can clone it as follows:
> 
>     hg clone http://hg.openjdk.java.net/jdk/sandbox
>     cd ./sandbox
>     hg update JDK-8200758-branch
> 
> I plan to reply to the feedback already provided, and update the JEP [2]
> next week some time, but in the mean time if you have additional
> questions or comments, feel free to reply.
> 
> -- Kevin
> 
> [1] http://hg.openjdk.java.net/jdk/sandbox/shortlog/JDK-8200758-branch
> [2] https://bugs.openjdk.java.net/browse/JDK-8200758
> 
> 
> On 6/27/2018 3:30 PM, Kevin Rushforth wrote:
>> We're aiming to get this into JDK 12 early enough so that an EA build
>> would be available around the time JDK 11 ships. That will allow you
>> to take a jlinked image with JDK 11 and package it up using (the EA)
>> jpackager.
>>
>> We will create a development branch in the JDK sandbox [1] some time
>> in the next week or so so you can follow the development.
>>
>> Also, thank you to those who have provided feedback. I'll reply to
>> feedback soon and then incorporate it into an updated JEP.
>>
> > -- Kevin

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

[Jiangli Zhou]

Thanks a lot for reviewing, Mandy!

Jiangli

> On Jul 6, 2018, at 1:40 PM, mandy chung  wrote:
> 
> Hi Jiangli,
> 
> On 6/28/18 4:15 PM, Jiangli Zhou wrote:> webrev: 
> http://cr.openjdk.java.net/~jiangli/8202035/webrev.00/
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8202035?filter=14921
> 
> Good work.  I'm glad to see a pretty good startup improvement.
> 
> I reviewed java.base change that looks good.
> 
> Mandy

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

[Jiangli Zhou]

> On Jul 6, 2018, at 1:36 PM, Ioi Lam  wrote:
> 
> Hi Jiangli,
> 
> The VM changes look good to me.

Thanks!

> 
> For the tests: I think we need a comment here saying that "mods" is 
> intentionally empty, and also an explanation why it's not necessary to 
> actually fill with actual modules?

Will do.

Thanks,
Jiangli

> 
> Thanks
> 
> - Ioi
> 
> 
>>> 3) ArchivedModuleComboTest.java
>>> 
>>>  55 Path moduleDir = Files.createTempDirectory(userDir, "mods");
>>> 
>>>  I don't see anything got placed under the "mods" dir, is it by design?
>> Yes.
>> 
> 
> 
> On 7/6/18 12:34 PM, Jiangli Zhou wrote:
>> Hi Calvin,
>> 
>> Thanks for the review! Here is the updated webrevs that address the 
>> feedbacks from you and Ioi:
>> 
>> http://cr.openjdk.java.net/~jiangli/8202035/webrev_inc.01/
>> 
>> Full webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev_full.01/
>> 
>>> On Jul 6, 2018, at 9:15 AM, Calvin Cheung  wrote:
>>> 
>>> Hi Jiangli,
>>> 
>>> Thanks for this start-up improvement. The changes look good overall. I've 
>>> the following minor comments.
>>> 
>>> 1) make/hotspot/symbols/symbols-unix
>>> 
>>>134 JVM_InitializeFromArchive
>>> 
>>>If you want the symbols to be in alphabetical order, the above should be 
>>> moved after JVM_InitStackTraceElementArray.
>> Fixed.
>> 
>>> 2) metaspaceShared.cpp
>>> 
>>>1927 oop MetaspaceShared::materialize_archived_object(oop obj) {
>>>1928   if (obj != NULL) {
>>>1929 return 
>>> G1CollectedHeap::heap()->materialize_archived_object(obj);
>>>1930   }
>>>1931   return NULL;
>>>1932 }
>>> 
>>>Instead of two return statements, how about replacing lines 1928 - 1931 
>>> with the following?
>>> 
>>>return (obj != NULL) ? 
>>> G1CollectedHeap::heap()->materialize_archived_object(obj) : NULL;
>> The original format probably is slightly easier to read, so I left it 
>> unchanged. Hope that’s okay with you.
>> 
>>> 3) ArchivedModuleComboTest.java
>>> 
>>>  55 Path moduleDir = Files.createTempDirectory(userDir, "mods");
>>> 
>>>  I don't see anything got placed under the "mods" dir, is it by design?
>> Yes.
>> 
>>>  For the "dump with --module-path" cases, there seems to be a missing test 
>>> case with "--show-module-resolution" (similar to Test case 2).
>> When --module-path is specified at dump time, system module graph is not 
>> archived currently. There is no need for additional test case with 
>> --show-module-resolution in this case since all module objects are created 
>> as normal.
>> 
>>> 
>>> 4) CheckArchivedModuleApp.java
>>> 
>>>  53 if (expectArchived && wb.isShared(md)) {
>>>  54 System.out.println(name + " is archived. Expected.");
>>>  55 } else if (!expectArchived && !wb.isShared(md)) {
>>>  56 System.out.println(name + " is not archived. 
>>> Expected.");
>>>  57 } else if (expectArchived) {
>>>  58 throw new RuntimeException(
>>>  59 "FAILED. " + name + " is not archived. Expect 
>>> archived.");
>>>  60 } else {
>>>  61 throw new RuntimeException(
>>>  62 "FAILED. " + name + " is archived. Expect not 
>>> archived.");
>>>  63 }
>>> 
>>>  I'd suggest the following so that the code is easier to understand:
>>> 
>>>  if (expectArchived) {
>>>  if (wb.isShared(md)) {
>>>  System.out.println(name + " is archived. Expected.");
>>>  } else {
>>>  throw new RuntimeException(
>>>  "FAILED. " + name + " is not archived. Expect archived.");
>>>  }
>>>  } else {
>>>  if (!wb.isShared(md)) {
>>>  System.out.println(name + " is not archived. Expected.");
>>>  } else {
>>>  throw new RuntimeException(
>>>  "FAILED. " + name + " is archived. Expect not archived.");
>>>  }
>>>  }
>> Reformatted as suggested.
>> 
>>> 5) ArchivedModuleWithCustomImageTest.java
>>> 
>>> 178 private static void printCommand(String opts[]) {
>>> 179 StringBuilder cmdLine = new StringBuilder();
>>> 180 for (String cmd : opts)
>>> 181 cmdLine.append(cmd).append(' ');
>>> 182 System.out.println("Command line: [" + cmdLine.toString() + 
>>> "]");
>>> 183 }
>>> 
>>> Consider putting the above method in ProcessTools.java so that 
>>> ProcessTools.createJavaProcessBuilder() and the above test can call it and 
>>> avoiding duplicate code.
>>> A separate follow-up bug to address this is fine.
>> That sounds good to me. We might need some reformatting for consolidation. I 
>> will file a follow-up RFE.
>> 
>>> 6) PrintSystemModulesApp.java
>>> 
>>>  I don't think it is being used?
>> It’s used by ArchivedModuleCompareTest.java. Looks like it was missing from 
>> the earlier webrev. Thanks for catching that. The file is included in the 
>> updated webrev.
>> 
>> Thanks!
>> Jiangli
>> 
>>> thanks,
>>> Calvin
>>> 
>>> On 6/28/18, 4:15 PM, Jiangli Zhou 

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

[mandy chung]

Hi Jiangli,

On 6/28/18 4:15 PM, Jiangli Zhou wrote:> webrev: 
http://cr.openjdk.java.net/~jiangli/8202035/webrev.00/

RFE: https://bugs.openjdk.java.net/browse/JDK-8202035?filter=14921


Good work.  I'm glad to see a pretty good startup improvement.

I reviewed java.base change that looks good.

Mandy

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

[Ioi Lam]

Hi Jiangli,

The VM changes look good to me.

For the tests: I think we need a comment here saying that "mods" is 
intentionally empty, and also an explanation why it's not necessary to 
actually fill with actual modules?


Thanks

- Ioi



3) ArchivedModuleComboTest.java

  55 Path moduleDir = Files.createTempDirectory(userDir, "mods");

  I don't see anything got placed under the "mods" dir, is it by design?

Yes.




On 7/6/18 12:34 PM, Jiangli Zhou wrote:

Hi Calvin,

Thanks for the review! Here is the updated webrevs that address the feedbacks 
from you and Ioi:

http://cr.openjdk.java.net/~jiangli/8202035/webrev_inc.01/

Full webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev_full.01/


On Jul 6, 2018, at 9:15 AM, Calvin Cheung  wrote:

Hi Jiangli,

Thanks for this start-up improvement. The changes look good overall. I've the 
following minor comments.

1) make/hotspot/symbols/symbols-unix

134 JVM_InitializeFromArchive

If you want the symbols to be in alphabetical order, the above should be 
moved after JVM_InitStackTraceElementArray.

Fixed.


2) metaspaceShared.cpp

1927 oop MetaspaceShared::materialize_archived_object(oop obj) {
1928   if (obj != NULL) {
1929 return G1CollectedHeap::heap()->materialize_archived_object(obj);
1930   }
1931   return NULL;
1932 }

Instead of two return statements, how about replacing lines 1928 - 1931 
with the following?

return (obj != NULL) ? 
G1CollectedHeap::heap()->materialize_archived_object(obj) : NULL;

The original format probably is slightly easier to read, so I left it 
unchanged. Hope that’s okay with you.


3) ArchivedModuleComboTest.java

  55 Path moduleDir = Files.createTempDirectory(userDir, "mods");

  I don't see anything got placed under the "mods" dir, is it by design?

Yes.


  For the "dump with --module-path" cases, there seems to be a missing test case with 
"--show-module-resolution" (similar to Test case 2).

When --module-path is specified at dump time, system module graph is not 
archived currently. There is no need for additional test case with 
--show-module-resolution in this case since all module objects are created as 
normal.



4) CheckArchivedModuleApp.java

  53 if (expectArchived && wb.isShared(md)) {
  54 System.out.println(name + " is archived. Expected.");
  55 } else if (!expectArchived && !wb.isShared(md)) {
  56 System.out.println(name + " is not archived. Expected.");
  57 } else if (expectArchived) {
  58 throw new RuntimeException(
  59 "FAILED. " + name + " is not archived. Expect 
archived.");
  60 } else {
  61 throw new RuntimeException(
  62 "FAILED. " + name + " is archived. Expect not 
archived.");
  63 }

  I'd suggest the following so that the code is easier to understand:

  if (expectArchived) {
  if (wb.isShared(md)) {
  System.out.println(name + " is archived. Expected.");
  } else {
  throw new RuntimeException(
  "FAILED. " + name + " is not archived. Expect archived.");
  }
  } else {
  if (!wb.isShared(md)) {
  System.out.println(name + " is not archived. Expected.");
  } else {
  throw new RuntimeException(
  "FAILED. " + name + " is archived. Expect not archived.");
  }
  }

Reformatted as suggested.


5) ArchivedModuleWithCustomImageTest.java

178 private static void printCommand(String opts[]) {
179 StringBuilder cmdLine = new StringBuilder();
180 for (String cmd : opts)
181 cmdLine.append(cmd).append(' ');
182 System.out.println("Command line: [" + cmdLine.toString() + "]");
183 }

Consider putting the above method in ProcessTools.java so that 
ProcessTools.createJavaProcessBuilder() and the above test can call it and 
avoiding duplicate code.
A separate follow-up bug to address this is fine.

That sounds good to me. We might need some reformatting for consolidation. I 
will file a follow-up RFE.


6) PrintSystemModulesApp.java

  I don't think it is being used?

It’s used by ArchivedModuleCompareTest.java. Looks like it was missing from the 
earlier webrev. Thanks for catching that. The file is included in the updated 
webrev.

Thanks!
Jiangli


thanks,
Calvin

On 6/28/18, 4:15 PM, Jiangli Zhou wrote:

This is a follow-up RFE of JDK-8201650 (Move iteration order randomization of 
unmodifiable Set and Map to iterators), which was resolved to allow Set/Map 
objects being archived at CDS dump time (thanks Claes and Stuart Marks). In the 
current RFE, it archives the set of system ModuleReference and ModuleDescriptor 
objects (including their referenced objects) in 'open' archive heap region at 
CDS dump time. It allows reusing of the objects and bypassing the process of 
creating the system ModuleDescriptors and ModuleReferences at 

Prototype of jpackager in jdk/sandbox [was: Draft JEP proposal: JDK-8200758: Packaging Tool]

[Kevin Rushforth]

An initial prototype of the jpackager tool has been pushed to a new 
'JDK-8200758-branch' branch in the JDK sandbox [1]. If anyone is 
interested in taking a look, you can clone it as follows:


    hg clone http://hg.openjdk.java.net/jdk/sandbox
    cd ./sandbox
    hg update JDK-8200758-branch

I plan to reply to the feedback already provided, and update the JEP [2] 
next week some time, but in the mean time if you have additional 
questions or comments, feel free to reply.


-- Kevin

[1] http://hg.openjdk.java.net/jdk/sandbox/shortlog/JDK-8200758-branch
[2] https://bugs.openjdk.java.net/browse/JDK-8200758


On 6/27/2018 3:30 PM, Kevin Rushforth wrote:
We're aiming to get this into JDK 12 early enough so that an EA build 
would be available around the time JDK 11 ships. That will allow you 
to take a jlinked image with JDK 11 and package it up using (the EA) 
jpackager.


We will create a development branch in the JDK sandbox [1] some time 
in the next week or so so you can follow the development.


Also, thank you to those who have provided feedback. I'll reply to 
feedback soon and then incorporate it into an updated JEP.


-- Kevin

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

[Jiangli Zhou]

Hi Calvin,

Thanks for the review! Here is the updated webrevs that address the feedbacks 
from you and Ioi:

http://cr.openjdk.java.net/~jiangli/8202035/webrev_inc.01/

Full webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev_full.01/

> On Jul 6, 2018, at 9:15 AM, Calvin Cheung  wrote:
> 
> Hi Jiangli,
> 
> Thanks for this start-up improvement. The changes look good overall. I've the 
> following minor comments.
> 
> 1) make/hotspot/symbols/symbols-unix
> 
>134 JVM_InitializeFromArchive
> 
>If you want the symbols to be in alphabetical order, the above should be 
> moved after JVM_InitStackTraceElementArray.

Fixed.

> 
> 2) metaspaceShared.cpp
> 
>1927 oop MetaspaceShared::materialize_archived_object(oop obj) {
>1928   if (obj != NULL) {
>1929 return G1CollectedHeap::heap()->materialize_archived_object(obj);
>1930   }
>1931   return NULL;
>1932 }
> 
>Instead of two return statements, how about replacing lines 1928 - 1931 
> with the following?
> 
>return (obj != NULL) ? 
> G1CollectedHeap::heap()->materialize_archived_object(obj) : NULL;

The original format probably is slightly easier to read, so I left it 
unchanged. Hope that’s okay with you.

> 
> 3) ArchivedModuleComboTest.java
> 
>  55 Path moduleDir = Files.createTempDirectory(userDir, "mods");
> 
>  I don't see anything got placed under the "mods" dir, is it by design?

Yes. 

> 
>  For the "dump with --module-path" cases, there seems to be a missing test 
> case with "--show-module-resolution" (similar to Test case 2).

When --module-path is specified at dump time, system module graph is not 
archived currently. There is no need for additional test case with 
--show-module-resolution in this case since all module objects are created as 
normal.

> 
> 
> 4) CheckArchivedModuleApp.java
> 
>  53 if (expectArchived && wb.isShared(md)) {
>  54 System.out.println(name + " is archived. Expected.");
>  55 } else if (!expectArchived && !wb.isShared(md)) {
>  56 System.out.println(name + " is not archived. Expected.");
>  57 } else if (expectArchived) {
>  58 throw new RuntimeException(
>  59 "FAILED. " + name + " is not archived. Expect 
> archived.");
>  60 } else {
>  61 throw new RuntimeException(
>  62 "FAILED. " + name + " is archived. Expect not 
> archived.");
>  63 }
> 
>  I'd suggest the following so that the code is easier to understand:
> 
>  if (expectArchived) {
>  if (wb.isShared(md)) {
>  System.out.println(name + " is archived. Expected.");
>  } else {
>  throw new RuntimeException(
>  "FAILED. " + name + " is not archived. Expect archived.");
>  }
>  } else {
>  if (!wb.isShared(md)) {
>  System.out.println(name + " is not archived. Expected.");
>  } else {
>  throw new RuntimeException(
>  "FAILED. " + name + " is archived. Expect not archived.");
>  }
>  }

Reformatted as suggested.

> 
> 5) ArchivedModuleWithCustomImageTest.java
> 
> 178 private static void printCommand(String opts[]) {
> 179 StringBuilder cmdLine = new StringBuilder();
> 180 for (String cmd : opts)
> 181 cmdLine.append(cmd).append(' ');
> 182 System.out.println("Command line: [" + cmdLine.toString() + "]");
> 183 }
> 
> Consider putting the above method in ProcessTools.java so that 
> ProcessTools.createJavaProcessBuilder() and the above test can call it and 
> avoiding duplicate code.
> A separate follow-up bug to address this is fine.

That sounds good to me. We might need some reformatting for consolidation. I 
will file a follow-up RFE.

> 
> 6) PrintSystemModulesApp.java
> 
>  I don't think it is being used?

It’s used by ArchivedModuleCompareTest.java. Looks like it was missing from the 
earlier webrev. Thanks for catching that. The file is included in the updated 
webrev.

Thanks!
Jiangli

> 
> thanks,
> Calvin
> 
> On 6/28/18, 4:15 PM, Jiangli Zhou wrote:
>> This is a follow-up RFE of JDK-8201650 (Move iteration order randomization 
>> of unmodifiable Set and Map to iterators), which was resolved to allow 
>> Set/Map objects being archived at CDS dump time (thanks Claes and Stuart 
>> Marks). In the current RFE, it archives the set of system ModuleReference 
>> and ModuleDescriptor objects (including their referenced objects) in 'open' 
>> archive heap region at CDS dump time. It allows reusing of the objects and 
>> bypassing the process of creating the system ModuleDescriptors and 
>> ModuleReferences at runtime for startup improvement. My preliminary 
>> measurements on linux-x64 showed ~5% startup improvement when running 
>> HelloWorld from -cp using archived module objects at runtime (without extra 
>> tuning).
>> 
>> The library changes in the following webrev are contributed by Alan Bateman. 
>> 

Re: RFR: 8206495 [testbug] Redundant System.setProperty(null) tests

[Lance Andersen]

Hi Roger,

Looks fine. 

> On Jul 6, 2018, at 3:08 PM, Roger Riggs  wrote:
> 
> Please review the removal of a duplicate test for System.setProperty(null).
> 
> Webrev:
>http://cr.openjdk.java.net/~rriggs/webrev-dup-prop-null/
> 
> Thanks, Roger
> 

 
  

 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: 8206495 [testbug] Redundant System.setProperty(null) tests

[mandy chung]

+1

Mandy

On 7/6/18 12:08 PM, Roger Riggs wrote:

Please review the removal of a duplicate test for System.setProperty(null).

Webrev:
    http://cr.openjdk.java.net/~rriggs/webrev-dup-prop-null/

Thanks, Roger

RFR: 8206495 [testbug] Redundant System.setProperty(null) tests

[Roger Riggs]

Please review the removal of a duplicate test for System.setProperty(null).

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-dup-prop-null/

Thanks, Roger

RFR: jsr166 integration 2018-07

[Martin Buchholz]

http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html

Some smallish changes; all of them already seen review elsewhere.

8206123: ArrayDeque created with default constructor can only hold 15
elements
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/ArrayDeque-capacity/index.html
https://bugs.openjdk.java.net/browse/JDK-8206123

8205576: forkjoin/FJExceptionTableLeak.java fails "AssertionError: failed
to satisfy condition"
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/FJExceptionTableLeak/index.html
https://bugs.openjdk.java.net/browse/JDK-8205576

8205620: Miscellaneous changes imported from jsr166 CVS 2018-07
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/miscellaneous/index.html
https://bugs.openjdk.java.net/browse/JDK-8205620

Re: RFR: CSR - JDK-8200435 String::align, String::indent (Preview)

[Jim Laskey]

Changed. Thank you.


> On Jul 6, 2018, at 2:31 PM, Roger Riggs  wrote:
> 
> Hi Jim,
> 
> The phrase:
> 
> "All Character#isWhitespace(int) white space characters, including tab, are 
> treated as a single space."
> 
> Can be mis-interpreted to mean collectively they are replaced by a single 
> space.
> 
> Perhaps replace "All" with "Each".
> 
> Regards, Roger
> 
> On 7/6/2018 1:16 PM, Jim Laskey wrote:
>> csr: https://bugs.openjdk.java.net/browse/JDK-8200435
> 

Re: RFR: CSR - JDK-8200435 String::align, String::indent (Preview)

[Roger Riggs]

Hi Jim,

The phrase:

"All Character#isWhitespace(int) white space characters, including tab, 
are treated as a single space."


Can be mis-interpreted to mean collectively they are replaced by a 
single space.


Perhaps replace "All" with "Each".

Regards, Roger

On 7/6/2018 1:16 PM, Jim Laskey wrote:

csr: https://bugs.openjdk.java.net/browse/JDK-8200435

RFR: CSR - JDK-8200435 String::align, String::indent (Preview)

[Jim Laskey]

csr: https://bugs.openjdk.java.net/browse/JDK-8200435

Re: RFR(L): 8202035: Archive the set of ModuleDescriptor and ModuleReference objects for system modules

[Calvin Cheung]

Hi Jiangli,

Thanks for this start-up improvement. The changes look good overall. 
I've the following minor comments.


1) make/hotspot/symbols/symbols-unix

134 JVM_InitializeFromArchive

If you want the symbols to be in alphabetical order, the above 
should be moved after JVM_InitStackTraceElementArray.


2) metaspaceShared.cpp

1927 oop MetaspaceShared::materialize_archived_object(oop obj) {
1928   if (obj != NULL) {
1929 return 
G1CollectedHeap::heap()->materialize_archived_object(obj);

1930   }
1931   return NULL;
1932 }

Instead of two return statements, how about replacing lines 1928 - 
1931 with the following?


return (obj != NULL) ? 
G1CollectedHeap::heap()->materialize_archived_object(obj) : NULL;


3) ArchivedModuleComboTest.java

  55 Path moduleDir = Files.createTempDirectory(userDir, "mods");

  I don't see anything got placed under the "mods" dir, is it by design?

  For the "dump with --module-path" cases, there seems to be a missing 
test case with "--show-module-resolution" (similar to Test case 2).



4) CheckArchivedModuleApp.java

  53 if (expectArchived && wb.isShared(md)) {
  54 System.out.println(name + " is archived. Expected.");
  55 } else if (!expectArchived && !wb.isShared(md)) {
  56 System.out.println(name + " is not archived. 
Expected.");

  57 } else if (expectArchived) {
  58 throw new RuntimeException(
  59 "FAILED. " + name + " is not archived. 
Expect archived.");

  60 } else {
  61 throw new RuntimeException(
  62 "FAILED. " + name + " is archived. Expect 
not archived.");

  63 }

  I'd suggest the following so that the code is easier to understand:

  if (expectArchived) {
  if (wb.isShared(md)) {
  System.out.println(name + " is archived. Expected.");
  } else {
  throw new RuntimeException(
  "FAILED. " + name + " is not archived. Expect archived.");
  }
  } else {
  if (!wb.isShared(md)) {
  System.out.println(name + " is not archived. Expected.");
  } else {
  throw new RuntimeException(
  "FAILED. " + name + " is archived. Expect not archived.");
  }
  }

5) ArchivedModuleWithCustomImageTest.java

 178 private static void printCommand(String opts[]) {
 179 StringBuilder cmdLine = new StringBuilder();
 180 for (String cmd : opts)
 181 cmdLine.append(cmd).append(' ');
 182 System.out.println("Command line: [" + cmdLine.toString() 
+ "]");

 183 }

 Consider putting the above method in ProcessTools.java so that 
ProcessTools.createJavaProcessBuilder() and the above test can call it 
and avoiding duplicate code.

 A separate follow-up bug to address this is fine.

6) PrintSystemModulesApp.java

  I don't think it is being used?

thanks,
Calvin

On 6/28/18, 4:15 PM, Jiangli Zhou wrote:

This is a follow-up RFE of JDK-8201650 (Move iteration order randomization of 
unmodifiable Set and Map to iterators), which was resolved to allow Set/Map 
objects being archived at CDS dump time (thanks Claes and Stuart Marks). In the 
current RFE, it archives the set of system ModuleReference and ModuleDescriptor 
objects (including their referenced objects) in 'open' archive heap region at 
CDS dump time. It allows reusing of the objects and bypassing the process of 
creating the system ModuleDescriptors and ModuleReferences at runtime for 
startup improvement. My preliminary measurements on linux-x64 showed ~5% 
startup improvement when running HelloWorld from -cp using archived module 
objects at runtime (without extra tuning).

The library changes in the following webrev are contributed by Alan Bateman. 
Thanks Alan and Mandy for discussions and help. Thanks Karen, Lois and Ioi for 
discussion and suggestions on initialization ordering.

The majority of the module object archiving code are in heapShared.hpp and 
heapShared.cpp. Thanks Coleen for pre-review and Eric Caspole for helping 
performance tests.

webrev: http://cr.openjdk.java.net/~jiangli/8202035/webrev.00/
RFE: https://bugs.openjdk.java.net/browse/JDK-8202035?filter=14921

Tested using tier1 - tier6 via mach5 including all new test cases added in the 
webrev.

Following are the details of system module archiving, which are duplicated in 
above bug report.
---
Support archiving system module graph when the initial module is unnamed module 
from -cp currently.

Support G1 GC, 64-bit (non-Windows). Requires UseCompressedOops and 
UseCompressedClassPointers.

Dump time system module object archiving
=
At dump time, the following fields in ArchivedModuleGraph are set to record the 
system module information created by 

Re: RFR(M): 8203826: Chain class initialization exceptions into later NoClassDefFoundErrors

[Peter Levart]

Hi,

On 07/05/2018 01:01 AM, David Holmes wrote:
I dispute "they will understand this might have happened in another 
thread". 


What if the stack trace was like the following...

Before patch:

1st attempt [ForkJoinPool.commonPool-worker-3]:

java.lang.ExceptionInInitializerError
    at ClinitFailure.lambda$main$0(ClinitFailure.java:20)
    at 
java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
    at 
java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
    at 
java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at 
java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
    at 
java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
    at 
java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
    at 
java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)

Caused by: java.lang.RuntimeException: Can't get it!
    at ClinitFailure$Faulty.(ClinitFailure.java:12)
    ... 8 more
Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of 
bounds for length 0

    at ClinitFailure$Faulty.(ClinitFailure.java:10)
    ... 8 more

2nd attempt [ForkJoinPool.commonPool-worker-5]:

java.lang.NoClassDefFoundError: Could not initialize class 
ClinitFailure$Faulty

    at ClinitFailure.lambda$main$1(ClinitFailure.java:28)
    at 
java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
    at 
java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
    at 
java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at 
java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
    at 
java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
    at 
java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
    at 
java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)



After patch:

1st attempt [ForkJoinPool.commonPool-worker-3]:

java.lang.ExceptionInInitializerError
    at ClinitFailure.lambda$main$0(ClinitFailure.java:18)
    at 
java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
    at 
java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)
    at 
java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at 
java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
    at 
java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
    at 
java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
    at 
java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)

Caused by: java.lang.RuntimeException: Can't get it!
    at ClinitFailure$Faulty.(ClinitFailure.java:10)
    ... 8 more
Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of 
bounds for length 0

    at ClinitFailure$Faulty.(ClinitFailure.java:8)
    ... 8 more

2nd attempt [ForkJoinPool.commonPool-worker-5]:

java.lang.NoClassDefFoundError: Could not initialize class 
ClinitFailure$Faulty
    at 
java.base/java.lang.ClassLoader.throwReinitException(ClassLoader.java:3062)

    at ClinitFailure.lambda$main$1(ClinitFailure.java:25)
    at 
java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:783)
    at 
java.base/java.util.concurrent.CompletableFuture$Completion.exec(CompletableFuture.java:479)
    at 
java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at 
java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
    at 
java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
    at 
java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
    at 
java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: java.lang.ExceptionInInitializerError: 11 ms ago in thread 
ForkJoinPool.commonPool-worker-3

    at ClinitFailure.lambda$main$0(ClinitFailure.java:18)
    at 
java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1736)
    at 
java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1728)

    ... 5 more
Caused by: java.lang.RuntimeException: Can't get it!
    at ClinitFailure$Faulty.(ClinitFailure.java:10)
    ... 8 more
Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 1 out of 
bounds for length 0

    at ClinitFailure$Faulty.(ClinitFailure.java:8)
    ... 8 more



This is what gets printed by the sample program:

Re: Adding new IBM extended charsets

[Nasser Ebrahim]

Hi Sherman,

Thank you for your valuable inputs. I would like to clarify some of your 
points.

> I would assume the best option might be (3), in which only keeps those
> ibm charsets that might be used/configured to be the default charset for
> vm startup on IBM platform, and leaves the rest to icu4j charsets 
provider.

I understood you preferred option is 3 [Remove all extended charsets from 
JDK (keep only default charsets) and use the extended charsets from third 
party like ICU4J]. Just to confirm, so you meant we need to keep only the 
standard charsets in the JDK and remove all the extended charsets from JDK 
and use them from ICU4J OR you meant apply that only for the new extended 
charsets. I think it is better to keep the consistency - either take all 
extended charsets from ICU4J or maintain all extended charsets with JDK. 
Keeping some extended charsets within JDK and use ICU4J for other extended 
charsets may confuse the Java user.

> There are hundreds of IBM specific charsets, and with various versions 
that
> probably are not going to be used by most of the Java developers in most
> normal use scenarios. I would assume it's NOT in our best interest to 
> implement,
> test and support/maintain them in openjdk project/repo.

As I explained in the previous note, the IBM charsets are not just 
applicable to IBM Platforms. It can be applicable to any platforms if Java 
has to access an application or database on IBM platforms. For example, if 
a Java application on Linux or Windows requires EBCDIC code pages for JDBC 
to access DB2 on z/OS.
 
Precisely, there are 75 charsets from IBM JDK which are currently missing 
in openjdk. Within the 75 missing charsets, 2 charsets are default 
charsets on AIX platform. Out of the remaining 73 charsets, some of them 
are default charsets for z/OS platform. We would like to contribute those 
75 charsets to openJDK. 
 
> For those to be kept in openjdk repo, they could/should be separated 
from
> the extended charsets and kept into a separate package/module as well, 
and
> probably are only configured to build into the binaries for IBM 
> platforms, and
> maintained by the AIX port.

Yes. I agree it makes sense to keep all IBM charsets as a separate charset 
provider / module and include them on a need basis to reduce the 
footprint. If everybody agrees, we can start working towards creating a 
separate charset provider/module for IBM charsets in JDK. Please advise. 
 
Thank you,
Nasser Ebrahim




From:   Xueming Shen 
To: core-libs-dev@openjdk.java.net
Date:   07/05/2018 10:14 PM
Subject:Re: Adding new IBM extended charsets
Sent by:"core-libs-dev" 



On 7/4/18, 5:41 AM, Nasser Ebrahim wrote:
> Hello,
>
> Am starting this mail thread to discuss about adding new IBM extended
> charsets. The questions is whether we need to add the new extended
> charsets to jdk.charsets or to a new separate charset provider/module 
like
> jdk.ibmcharsets. This discussion is in continuation of the suggestion 
from
> Alan Bateman in the mail chain -
> 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-May/053316.html
.
>
>
> Am copying his inputs from that mail thread to start the discussion:
> "I think we should start a discussion here about moving some or all of 
the
> IBM charsets to their own service provider module. I realize the AIX 
port
> might want to include some of them in its build of java.base but they
> aren't interesting to include in java.base, or even jdk.charsets, on 
most
> platform"
>
> First, let me clarify whether IBM charsets are applicable only to IBM
> platforms like AIX or applicable to other platforms as well. All IBM
> charsets are applicable to any platforms including Linux and windows if
> those platforms needs to communicate with an application or database in
> IBM platforms like AIX. That is the reason, we traditionally add them to
> the jdk.charsets. However, we agree with Alan that those IBM charsets 
are
> not required if the JDK is not communicating to any 
applications/databases
> on IBM platforms. Hence, it makes sense to consider a separate charset
> provider / module for IBM charsets and use build parameters to decide
> whether to generate the new charset provider or not for any platforms.
>
> Let me list out all the possible options I can think of for adding new
> extended charsets so that we can discuss and decide which is the best
> option.
>
> 1) Continue to add new extended charsets to jdk.charsets.
> The advantage with this approach is that no need to add new charset
> provider and all extended charsets are placed in one module. Also, any
> extended charset is applicable to any platform if they need to 
communicate
> with application/database in different platforms. The disadvantage is 
that
> the number of charsets in jdk.charsets keep increasing and blot its 
size.
> Also, many of those charsets may not be used in the lifetime of the JDK
> unless it is communicating  with application/databases of 

RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

[Baesken, Matthias]

Hi Alan ,so it looks likeJDK-8204233  added  a switch  (system property)  
to enable the enhanced  socket IOException messages .

That would be an option as well  for  8205525 .

8205525  adds  the  jar file name  and   the line number  info  to the 
exception message .

In case that only  the jar file name  would be considered sensitive ,   I would 
prefer to   just  output  the line number  (and omit the  system property ).
What do you think ?

Best regards, Matthias


> -Original Message-
> From: Alan Bateman [mailto:alan.bate...@oracle.com]
> Sent: Montag, 25. Juni 2018 16:52
> To: Baesken, Matthias ; core-libs-
> d...@openjdk.java.net
> Cc: Lindenmaier, Goetz 
> Subject: Re: [RFR] 8205525 : Improve exception messages during manifest
> parsing of jar archives
> 
> On 25/06/2018 15:29, Baesken, Matthias wrote:
> > Hi, do you consider  both  the file name and  line number as sensitive ?
> >
> >> There was a similar discussion on net-dev recently related to
> >> leaking host names in exceptions. Something similar may be needed here
> >>
> > Do you know the outcome of this discussion ?
> >
> All the details are in JDK-8204233 and the associated CSR.
> 
> -Alan

Re: Adding new IBM extended charsets

[Nasser Ebrahim]

Hi Florian,

Thank you for your response. iconv is platform dependent and not good for 
the platform agnostic nature of Java. Also, many charsets in Java are not 
available across platforms. I believe Java decided to have its own 
charsets due to those reasons so that it can work seamlessly on any 
supported platforms.

Thank you,
Nasser Ebrahim 



From:   Florian Weimer 
To: Nasser Ebrahim , Java Core Libs 

Date:   07/04/2018 07:11 PM
Subject:Re: Adding new IBM extended charsets



On 07/04/2018 02:41 PM, Nasser Ebrahim wrote:
> Please share your thoughts on your preferred option and list out any 
other
> options which I missed out. Thank you for your time.

Could you use the platform iconv implementation instead?  That would 
avoid shipping the tables in the JDK.

Thanks,
Florian

RFR[jdk8u-dev]: 8134124: sun/security/tools/jarsigner/warnings.sh fails when using Hindi locale

[Ramanand Patil]

Hi all,
Please review this trivial test fix:
Bug: https://bugs.openjdk.java.net/browse/JDK-8134124
Webrev: http://cr.openjdk.java.net/~rpatil/8134124/webrev.00/

The test is made locale independent now.

Regards,
Ramanand.

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

[Ichiroh Takiguchi]

Hello.
I'm not reviewer, but I tested your fix with some locales (non-UTF8 
locales) on AIX platform.


$ LANG=C javac TestIBMBugs.java
$ LANG=ko_KR javac TestIBMBugs.java
$ LANG=zh_CN javac TestIBMBugs.java

Also testcase worked fine as expected.

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

On 2018-07-06 15:56, Volker Simonis wrote:

Hi,

can I please have a review for this trivial test fix?

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

The problem is that the test contains some non-ASCII characters in
comments which makes the compilation fail for non-Unicode locales.

Thank you and best regards,
Volker

Re: [11] RFR: 8198405: JImageExtractTest.java & JImageListTest.java failed in Windows.

[David Holmes]

On 6/07/2018 6:03 PM, Srinivas Dama wrote:

Hi,

This patch fixes JImageExtractTest.java but nothing done specific to 
JImageListTest.java as
I couldn't reproduce this failure earlier.
So I just removed JImageListTest.java from ProblemList.txt.

Now I am reverting this change and raised a new bug specific to this 
failure[JDK-8206445].

Please review this trivial fix to revert ProblemList.txt
webrev: http://cr.openjdk.java.net/~sdama/8206450/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8206450


That looks okay.

Thanks,
David


Regards,
Srinivas
- Original Message -
From: david.hol...@oracle.com
To: james.las...@oracle.com, srinivas.d...@oracle.com
Cc: core-libs-dev@openjdk.java.net
Sent: Friday, 6 July, 2018 6:56:00 AM GMT +05:30 Chennai, Kolkata, Mumbai, New 
Delhi
Subject: Re: [11] RFR: 8198405: JImageExtractTest.java & JImageListTest.java 
failed in Windows.

The test still fails in JDK 11 CI tier 2 after this patch was applied.
See link in bug report.

David

On 6/07/2018 3:44 AM, Jim Laskey wrote:

+1


On Jul 5, 2018, at 2:42 PM, Srinivas Dama  wrote:

Hi,

Please review
Webrev: http://cr.openjdk.java.net/~sdama/8198405/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8198405

Regards,
Srinivas

Re: [11] RFR: 8198405: JImageExtractTest.java & JImageListTest.java failed in Windows.

[Srinivas Dama]

Hi,

This patch fixes JImageExtractTest.java but nothing done specific to 
JImageListTest.java as
I couldn't reproduce this failure earlier. 
So I just removed JImageListTest.java from ProblemList.txt.

Now I am reverting this change and raised a new bug specific to this 
failure[JDK-8206445].

Please review this trivial fix to revert ProblemList.txt
webrev: http://cr.openjdk.java.net/~sdama/8206450/webrev.00/
bug: https://bugs.openjdk.java.net/browse/JDK-8206450

Regards,
Srinivas
- Original Message -
From: david.hol...@oracle.com
To: james.las...@oracle.com, srinivas.d...@oracle.com
Cc: core-libs-dev@openjdk.java.net
Sent: Friday, 6 July, 2018 6:56:00 AM GMT +05:30 Chennai, Kolkata, Mumbai, New 
Delhi
Subject: Re: [11] RFR: 8198405: JImageExtractTest.java & JImageListTest.java 
failed in Windows.

The test still fails in JDK 11 CI tier 2 after this patch was applied. 
See link in bug report.

David

On 6/07/2018 3:44 AM, Jim Laskey wrote:
> +1
> 
>> On Jul 5, 2018, at 2:42 PM, Srinivas Dama  wrote:
>>
>> Hi,
>>
>> Please review
>> Webrev: http://cr.openjdk.java.net/~sdama/8198405/webrev.00/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8198405
>>
>> Regards,
>> Srinivas
> 

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

[Volker Simonis]

Hi Mikael, Thomas, Alan,

thanks a lot for the quick reviews.
I've just fixed the typo and pushed the fix.

Sorry for the trouble,
Volker


On Fri, Jul 6, 2018 at 9:06 AM, Alan Bateman  wrote:
> On 06/07/2018 08:03, Mikael Vidstedt wrote:
>>
>> Fix the typo while at it?
>>
>> +// and [yen] an [overscore] are encoded to 0x5c and 0x7e
>>
>> an -> and
>>
>>
> I agree, otherwise looks okay to me.
>
> -Alan

Re: JDK 12 RFR of JDK-8206440: Remove javac -source/-target 6 from jdk regression tests

[Alan Bateman]

On 06/07/2018 04:37, joe darcy wrote:

Hello,

With javac support for -source/-target 6 and 1.6 planned to be removed 
later in JDK 12 (JDK-8028563), please review the removal of usage of 
these options in the non-manual jdk regression tests:

This looks okay.

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

[Alan Bateman]

On 06/07/2018 08:03, Mikael Vidstedt wrote:

Fix the typo while at it?

+// and [yen] an [overscore] are encoded to 0x5c and 0x7e

an -> and



I agree, otherwise looks okay to me.

-Alan

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

[Thomas Stüfe]

Hi Volker, looks good and trivial. I did not test-compile it, but I
trust you did that already.

Best Regards, Thomas

On Fri, Jul 6, 2018 at 8:56 AM, Volker Simonis  wrote:
> Hi,
>
> can I please have a review for this trivial test fix?
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8206436/
> https://bugs.openjdk.java.net/browse/JDK-8206436
>
> The problem is that the test contains some non-ASCII characters in
> comments which makes the compilation fail for non-Unicode locales.
>
> Thank you and best regards,
> Volker

Re: [11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

[Mikael Vidstedt]

Fix the typo while at it?

+// and [yen] an [overscore] are encoded to 0x5c and 0x7e

an -> and

Cheers,
Mikael


> On Jul 5, 2018, at 11:56 PM, Volker Simonis  wrote:
> 
> Hi,
> 
> can I please have a review for this trivial test fix?
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8206436/
> https://bugs.openjdk.java.net/browse/JDK-8206436
> 
> The problem is that the test contains some non-ASCII characters in
> comments which makes the compilation fail for non-Unicode locales.
> 
> Thank you and best regards,
> Volker

[11] RFR(XXS): 8206436: sun/nio/cs/TestIBMBugs.java no longer compiles

[Volker Simonis]

Hi,

can I please have a review for this trivial test fix?

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

The problem is that the test contains some non-ASCII characters in
comments which makes the compilation fail for non-Unicode locales.

Thank you and best regards,
Volker