Re: Improving ZipFile.getEntry performance using Bloom filters

2020-04-14 Thread Ioi Lam
On 4/13/20 12:26 PM, Eirik Bjørsnøs wrote: Hi, While working on improvements to JarFile.getVersionedEntry, it occurred to me that the missing lookups we are removing there may be just one example of a more general issue. To get a better understanding of how ZipFile.getEntry is used, I added

RFR(S) 8241071 Generation of classes.jsa is not deterministic

2020-04-26 Thread Ioi Lam
https://bugs.openjdk.java.net/browse/JDK-8241071 http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v02/ The goal is to for "java -Xshare:dump" to produce deterministic contents in the CDS archive that depend only on the contents of $JAVA_HOME/lib/modules. Problems: [1] Sy

Re: RFR(S) 8241071 Generation of classes.jsa is not deterministic

2020-04-27 Thread Ioi Lam
3.delta/ More comments below On 4/26/20 11:20 PM, David Holmes wrote: Hi Ioi, On 27/04/2020 3:22 pm, Ioi Lam wrote: https://bugs.openjdk.java.net/browse/JDK-8241071 http://cr.openjdk.java.net/~iklam/jdk15/8241071-deterministic-cds-archive.v02/ The goal is to for "java -Xshare:dump"

Re: RFR(S) 8241071 Generation of classes.jsa with -Xshare:dump is not deterministic

2020-04-27 Thread Ioi Lam
uld just reserve a range and commit it manually as we go. Or could we not order the placement of Klass and Symbol at dump time? Dump time is not that time critical, no? Thanks & Sorry, Thomas On Mon, Apr 27, 2020 at 7:31 AM Ioi Lam mailto:ioi@oracle.com&g

Re: RFR(S) 8241815: Unnecessary calls to SystemDictionaryShared::define_shared_package

2020-04-27 Thread Ioi Lam
Hi Calvin, this looks good to me, too. Thanks - Ioi On 4/27/20 12:21 PM, Calvin Cheung wrote: JBS: https://bugs.openjdk.java.net/browse/JDK-8241815 webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8241815/webrev.00/ This change is to avoid java up call of definePackage() while loading a shar

Re: RFR(T) : 8244066 : ClassFileInstaller should be run in driver mode

2020-04-28 Thread Ioi Lam
Looks good and trivial. Thanks - Ioi On 4/28/20 9:41 PM, Igor Ignatyev wrote: http://cr.openjdk.java.net/~iignatyev//8244066/webrev.00 16 lines changed: 0 ins; 0 del; 16 mod; Hi all, could you please review this trivial cleanup in tests? from JBS: ClassFileInstaller is a test utility class

Re: RFR(S) 8241071 Generation of classes.jsa is not deterministic

2020-05-04 Thread Ioi Lam
On 4/27/20 7:05 PM, David Holmes wrote: On 28/04/2020 10:01 am, David Holmes wrote: On 28/04/2020 9:37 am, Ioi Lam wrote: Hi David & Jiangli, Thanks for your comments. I thought about using a system property, but the user can also specify it like java -Djdk.xshare.dump.sa

Re: Build error with GCC 10 in NetworkInterface.c and k_standard.c

2020-07-21 Thread Ioi Lam
On 7/17/20 6:48 AM, Yasumasa Suenaga wrote: Hi Koichi, On 2020/07/17 20:26, Koichi Sakata wrote: Hi Daniel,  > The changes to NetworkInterface.c look good to me. Thank you.  > You'll need to find a reviewer that understands what that  > method is supposed to do in that case, that's not me

RFR(L) 8244778 Archive full module graph in CDS

2020-07-22 Thread Ioi Lam
https://bugs.openjdk.java.net/browse/JDK-8244778 http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v01/ Please review this patch that stores the full module graph in the CDS archive heap. This reduces the initialization time of the basic JVM by about 22%: $ perf stat -r 1

Re: Fwd: RFR: 8247536: Support pre-generated MethodHandle lambda forms in CDS

2020-08-11 Thread Ioi Lam
Hi Yumin, This looks good! Here are my comments: [1] classListParser.cpp: #define LAMBDA_FOMRM_INVOKER "@lambda-form-invoker" I think this should be moved to classListParser.hpp:     const char* lambda_form_invoker_tag() {     return "@lambda-form-invoker";     } Also, instead of also d

Re: RFR(L) 8244778 Archive full module graph in CDS

2020-08-12 Thread Ioi Lam
Overall looks promising.  I have some review comments below, but not a complete review.  I concentrated on the JVM side primarily how the archived module graph is read in, how the ModuleEntry and PackageEntry tables are created from the archive for the 3 builtin loaders and potential timing is

RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-08-15 Thread Ioi Lam
To better capture what we're trying to do in this RFE, I've changed the RFE title (and the subject of this email thread) to https://bugs.openjdk.java.net/browse/JDK-8247536 Support for pre-generated java.lang.invoke classes in CDS static archive I also update the RFE Description to give an over

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-08-15 Thread Ioi Lam
On 8/15/20 6:19 PM, Ioi Lam wrote: To better capture what we're trying to do in this RFE, I've changed the RFE title (and the subject of this email thread) to https://bugs.openjdk.java.net/browse/JDK-8247536 Support for pre-generated java.lang.invoke classes in CDS static archi

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-08-24 Thread Ioi Lam
On 8/20/20 5:10 PM, Mandy Chung wrote: On 8/19/20 10:14 PM, Yumin Qi wrote: HI, Mandy   Thanks for the review, I took one day off yesterday so just got a detail look of your reply. On 8/19/20 1:30 PM, Mandy Chung wrote: On 8/17/20 12:37 PM, Yumin Qi wrote: Hi, Ioi   Thanks for revie

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-08-24 Thread Ioi Lam
Hi Yumin, This looks good overall. Here are my comments: = 6065 size_t new_id = Atomic::add(&counter, (size_t)1); 6066 jio_snprintf(addr_buf, 20, INTPTR_FORMAT, new_id); I think this should be SIZE_FORMAT =   65 class KlassFactory : AllStatic {  

Re: RFR(L) 8244778 Archive full module graph in CDS

2020-08-31 Thread Ioi Lam
-graph.v03/ http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v03.delta/ Please see my comments in-line. On 8/25/20 7:58 AM, Lois Foltan wrote: Hi Ioi, Changes looks really good.  Comments interspersed below. Thanks, Lois On 8/12/2020 6:06 PM, Ioi Lam wrote: Hi Lois

RFR: 8244778: Archive full module graph in CDS

2020-09-08 Thread Ioi Lam
This is the same patch as [8244778-archive-full-module-graph.v03](http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v03/) published in [hotspot-runtime-...@openjdk.java.net](https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041496.html). The rest of th

Re: RFR: 8244778: Archive full module graph in CDS

2020-09-08 Thread Ioi Lam
On Tue, 8 Sep 2020 15:59:33 GMT, Ioi Lam wrote: > This is the same patch as > [8244778-archive-full-module-graph.v03](http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v03/) > published in > [hotspot-runtime-...@openjdk.java.net](https://mail.openjdk.java.n

Re: RFR: 8244778: Archive full module graph in CDS [v2]

2020-09-09 Thread Ioi Lam
html). > The rest of the review will continue on GitHub. I will add new commits to > respond to comments to the above e-mail. Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merg

Re: RFR: 8244778: Archive full module graph in CDS [v2]

2020-09-09 Thread Ioi Lam
On Wed, 9 Sep 2020 18:41:25 GMT, Lois Foltan wrote: >> Ioi Lam has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes >> the unrelated changes brought in by the merge/rebase. The pull request >> contains six

Re: RFR: 8244778: Archive full module graph in CDS [v3]

2020-09-09 Thread Ioi Lam
html). > The rest of the review will continue on GitHub. I will add new commits to > respond to comments to the above e-mail. Ioi Lam has updated the pull request incrementally with one additional commit since the last revision: Feedback from Coleen - Chang

Re: RFR: 8244778: Archive full module graph in CDS [v3]

2020-09-09 Thread Ioi Lam
On Tue, 8 Sep 2020 17:32:44 GMT, Coleen Phillimore wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Feedback from Coleen > > src/hotspot/share/classfile/classLoaderDataShared.cpp line 17

Re: RFR: 8244778: Archive full module graph in CDS [v4]

2020-09-11 Thread Ioi Lam
html). > The rest of the review will continue on GitHub. I will add new commits to > respond to comments to the above e-mail. Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merg

Re: RFR: 8244778: Archive full module graph in CDS [v4]

2020-09-12 Thread Ioi Lam
On Sat, 12 Sep 2020 12:34:06 GMT, Claes Redestad wrote: >> Ioi Lam has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes >> the unrelated changes brought in by the merge/rebase. The pull request >> contains

Re: RFR: 8244778: Archive full module graph in CDS [v5]

2020-09-12 Thread Ioi Lam
html). > The rest of the review will continue on GitHub. I will add new commits to > respond to comments to the above e-mail. Ioi Lam has updated the pull request incrementally with one additional commit since the last revision: Claes feedback (typos, refactored archived fields init);

Re: RFR: 8244778: Archive full module graph in CDS [v6]

2020-09-12 Thread Ioi Lam
html). > The rest of the review will continue on GitHub. I will add new commits to > respond to comments to the above e-mail. Ioi Lam has updated the pull request incrementally with one additional commit since the last revision: fixed failure in runtime/cds/DeterministicDump.java

Integrated: 8244778: Archive full module graph in CDS

2020-09-13 Thread Ioi Lam
On Tue, 8 Sep 2020 15:59:33 GMT, Ioi Lam wrote: > This is the same patch as > [8244778-archive-full-module-graph.v03](http://cr.openjdk.java.net/~iklam/jdk16/8244778-archive-full-module-graph.v03/) > published in > [hotspot-runtime-...@openjdk.java.net](https://mail.openjdk.java.n

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive

2020-09-16 Thread Ioi Lam
On Tue, 15 Sep 2020 18:57:55 GMT, Yumin Qi wrote: > This patch is reorganized after 8252725, which is separated from this patch > to refactor jlink glugin code. The previous > webrev with hg can be found at: > http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-05. With 8252725 > integrated,

Re: RFR: 8253208: Move CDS related code to a separate class

2020-09-19 Thread Ioi Lam
On Fri, 18 Sep 2020 23:47:56 GMT, Yumin Qi wrote: > With more CDS related code added to VM, it is time to move CDS code to a > separate class. CDS is the new class which is > specific to CDS. > Tests: tier1-4 src/java.base/share/classes/jdk/internal/misc/CDS.java line 52: > 50: * Check if

Re: RFR: 8253208: Move CDS related code to a separate class [v2]

2020-09-21 Thread Ioi Lam
On Mon, 21 Sep 2020 18:17:55 GMT, Yumin Qi wrote: >> With more CDS related code added to VM, it is time to move CDS code to a >> separate class. CDS is the new class which is >> specific to CDS. >> Tests: tier1-4 > > Yumin Qi has updated the pull request incrementally with one additional > comm

RFR: 8253496: [BACKOUT] JDK-8253208 Move CDS related code to a separate class

2020-09-22 Thread Ioi Lam
Please review this BACKOUT for tier-1 failures. I have tested all CDS tests locally on Linux-x64, including the failed DeterministicDump.java. I would like to apply for the "trivial" rule since this is a simple backout for tier-1 failures. I executed the command: git revert c1df13b855984dc2197

Integrated: 8253496: [BACKOUT] JDK-8253208 Move CDS related code to a separate class

2020-09-22 Thread Ioi Lam
On Tue, 22 Sep 2020 19:48:33 GMT, Ioi Lam wrote: > Please review this BACKOUT for tier-1 failures. I have tested all CDS tests > locally on Linux-x64, including the failed > DeterministicDump.java. > I would like to apply for the "trivial" rule since this is a simple

Re: RFR: 8253500: [REDO] JDK-8253208 Move CDS related code to a separate class

2020-09-23 Thread Ioi Lam
On Wed, 23 Sep 2020 23:05:59 GMT, Yumin Qi wrote: > This patch is a REDO for JDK-8253208 which was backed out since it caused > runtime/cds/DeterministicDump.java failed, > see JDK-8253495. Since the failure is another issue and only triggered by > this patch, the test case now is put on > Prob

Re: RFR: 8247536: Support for pre-generated java.lang.invoke classes in CDS static archive [v3]

2020-09-25 Thread Ioi Lam
On Fri, 25 Sep 2020 21:19:39 GMT, Yumin Qi wrote: >> This patch is reorganized after 8252725, which is separated from this patch >> to refactor jlink glugin code. The previous >> webrev with hg can be found at: >> http://cr.openjdk.java.net/~minqi/2020/8247536/webrev-05. With 8252725 >> integr

Re: RFR: 8247666: Support Lambda proxy classes in static CDS archive [v5]

2020-10-13 Thread Ioi Lam
On Tue, 13 Oct 2020 23:08:22 GMT, Calvin Cheung wrote: >> Following up on archiving lambda proxy classes in dynamic CDS archive >> ([JDK-8198698](https://bugs.openjdk.java.net/browse/JDK-8198698)), this RFE >> adds the functionality of archiving of >> lambda proxy classes in static CDS archive.

Re: RFR: 8254192: ExtraSharedClassListFile contains extra white space at end of line [v2]

2020-10-14 Thread Ioi Lam
On Thu, 15 Oct 2020 01:04:25 GMT, Yumin Qi wrote: >> Hi, Please review the simple change. >> >> The removing of white spaces from end of line in ClassListParser should be >> moved to before we add lambda-form-invoker >> line to array. After made such arrangement, changed CDS.java for trimm

Re: RFR: 8247666: Support Lambda proxy classes in static CDS archive [v9]

2020-10-19 Thread Ioi Lam
On Mon, 19 Oct 2020 16:04:31 GMT, Calvin Cheung wrote: >> Following up on archiving lambda proxy classes in dynamic CDS archive >> ([JDK-8198698](https://bugs.openjdk.java.net/browse/JDK-8198698)), this RFE >> adds the functionality of archiving of >> lambda proxy classes in static CDS archive.

Re: RFR(s): 8205131: remove Runtime trace methods

2019-06-05 Thread Ioi Lam
On 6/4/19 7:46 PM, Stuart Marks wrote: Hi all, Please review this changeset and CSR request that remove the traceInstructions() and traceMethodCalls() methods from java.lang.Runtime. These methods have been deprecated for removal since Java 9, and they do nothing. I've also removed a coupl

Re: RFR: 8225648:[TESTBUG] java/lang/annotation/loaderLeak/Main.java fails with -Xcomp

2019-06-13 Thread Ioi Lam
Hi Jie, Instead of using an obscure call Reference.reachabilityFence(loader), how about just making "loader" a static field in the test class, so it will be kept alive until it's explicitly set to null? Thanks - Ioi On 6/12/19 1:37 AM, Jie Fu wrote: Hi all, JBS:    https://bugs.openjdk.jav

Re: RFR: JDK-8222373 Improve CDS performance for custom class loaders

2019-06-20 Thread Ioi Lam
On 6/19/19 11:36 PM, Alan Bateman wrote: On 20/06/2019 02:36, yumin qi wrote: Hi, Please review: bug: https://bugs.openjdk.java.net/browse/JDK-8222373 webrev: http://cr.openjdk.java.net/~minqi/8222373/01/ To load shared class from CDS, first class file stream is read from jar file, then load

Re: RFR: JDK-8222373 Improve CDS performance for custom class loaders

2019-06-20 Thread Ioi Lam
On 6/20/19 10:35 AM, Alan Bateman wrote: On 20/06/2019 17:50, yumin qi wrote: Hi, Alan and Ioi   Thanks. Forget to add core-libs-dev for the review.   If add a public API, surely it should be discussed in detail the design, implementation and effects. One question, will adding a public API

Loading classes during VM shutdown

2019-10-18 Thread Ioi Lam
For JDK-8232081 (Try to link all classes with ArchiveClassesAtExit), when creating a dynamic CDS archive, I need to link all classes in the "loaded" state. I am thinking of doing it at this point: src/java.base/share/classes/java/lang/Shutdown.java:     private static void runHooks() {    

Re: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

2019-10-22 Thread Ioi Lam
Hi Adam, The HotSpot logging option:   -Xlog:class+load=debug will show the class loader, super class, interfaces, size of the classfile, etc. A related option:   -Xlog:class+resolve=debug shows all the class resolution done by the Java code during execution. You're right that the

Re: RFR: JDK-8232773: ClassLoading Debug Output for Specific Classes

2019-10-29 Thread Ioi Lam
opinions are appreciated. Bug:https://bugs.openjdk.java.net/browse/JDK-8232773 Rough example webrev:http://cr.openjdk.java.net/~afarley/8232773/webrev/ Best Regards Adam Farley IBM Runtimes From:   David Holmes To: Martin Buchholz Cc: Ioi Lam, core-libs-dev , Adam Farley8 Date:   24/10/20

Re: Should Java support ERROR_NO_MORE_FILES when canonicalizing paths on Windows?

2019-11-18 Thread Ioi Lam
Hi Thorsten, since you have problems filing bugs on JBS, I filed the following bug on your behalf. https://bugs.openjdk.java.net/browse/JDK-8234363 I have not investigated the issue in detail yet. How often do you see ERROR_NO_MORE_FILES happening? Have you checked if your process (apache?) h

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-02-26 Thread Ioi Lam
On 2/26/20 7:50 PM, David Holmes wrote: Hi Calvin, Adding core-libs-dev as you are messing with their code :) On 27/02/2020 1:19 pm, Calvin Cheung wrote: JBS: https://bugs.openjdk.java.net/browse/JDK-8232081 webrev: http://cr.openjdk.java.net/~ccheung/jdk15/8232081/webrev.00/ The proposed

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-02-27 Thread Ioi Lam
re never linked when the application actually executed ?? On 28/02/2020 10:48 am, Calvin Cheung wrote: Hi David, Ioi, Thanks for your review and suggestions. On 2/26/20 9:46 PM, Ioi Lam wrote: On 2/26/20 7:50 PM, David Holmes wrote: Hi Calvin, Adding core-libs-dev as you are messing wit

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-02-27 Thread Ioi Lam
/20 4:48 PM, Calvin Cheung wrote: Hi David, Ioi, Thanks for your review and suggestions. On 2/26/20 9:46 PM, Ioi Lam wrote: On 2/26/20 7:50 PM, David Holmes wrote: Hi Calvin, Adding core-libs-dev as you are messing with their code :) On 27/02/2020 1:19 pm, Calvin Cheung wrote: JBS:

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-02-27 Thread Ioi Lam
On 2/27/20 10:14 PM, Calvin Cheung wrote: Hi Ioi, On 2/27/20 8:22 PM, Ioi Lam wrote: Hi Calvin, This looks good to me. Thanks for taking another look. I would suggest adding the something like this to the test case: class Child extends Parent {     int get() {return 2;}     int dummy

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-03-05 Thread Ioi Lam
the above. I'll try to answer your questions inline below.. Responses inline below ... On 28/02/2020 10:48 am, Calvin Cheung wrote: Hi David, Ioi, Thanks for your review and suggestions. On 2/26/20 9:46 PM, Ioi Lam wrote: On 2/26/20 7:50 PM, David Holmes wrote: Hi Calvin, Adding co

Re: RFR(S): 8232081: Try to link all classes during dynamic CDS dump

2020-03-05 Thread Ioi Lam
eed for new webrev. Thanks - Ioi On 3/5/20 11:48 AM, Calvin Cheung wrote: On 3/5/20 9:17 AM, Ioi Lam wrote: Hi Calvin, Looks good overall. I just have two comment: I think this is not needed:  306 void ConstantPool::resolve_class_constants(TRAPS) {  307   Arguments::assert_is_dumping_archive

Re: RFR(XS): 8174768: Make ProcessTools print executed process output into a separate file

2020-04-01 Thread Ioi Lam
On 4/1/20 12:13 PM, Igor Ignatyev wrote: Hi Evgeny, (widening the audience, given this affects not just hotspot compiler, but hotspot tests as well as core libs tests in general) overall that looks good to me. one suggestion, for the ease of failure analysis it might be worth to print out

Re: RFR: 8257241: CDS should not handle disableEagerInitialization for archived lambda proxy classes

2020-12-01 Thread Ioi Lam
On Tue, 1 Dec 2020 19:30:45 GMT, Calvin Cheung wrote: > Please review this change which includes: > > - If the `jdk.internal.lambda.disableEagerInitialization`property is enabled, > the `InnerClassLambdaMetafactory` will not involve CDS to archive lambda > proxy classes or to find them from an

Re: RFR: 8257241: CDS should not handle disableEagerInitialization for archived lambda proxy classes [v2]

2020-12-01 Thread Ioi Lam
On Tue, 1 Dec 2020 22:34:58 GMT, Calvin Cheung wrote: >> test/hotspot/jtreg/runtime/cds/appcds/LambdaEagerInit.java line 36: >> >>> 34: * @requires vm.cds >>> 35: * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds >>> 36: * @compile >>> ../../../../../jdk/java/lang/invoke/lambda/Lam

Re: RFR: 8257241: CDS should not handle disableEagerInitialization for archived lambda proxy classes [v4]

2020-12-01 Thread Ioi Lam
On Wed, 2 Dec 2020 01:12:17 GMT, Calvin Cheung wrote: >> Please review this change which includes: >> >> - If the `jdk.internal.lambda.disableEagerInitialization`property is >> enabled, the `InnerClassLambdaMetafactory` will not involve CDS to archive >> lambda proxy classes or to find them fr

RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX

2021-02-01 Thread Ioi Lam
- JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which allowed the same JDK library to use different version of HotSpot. However, HSX is no longer supported so this API should be removed. - Implementations of APIs such as JVM_DTraceActivate, were removed in [JDK-8068976](https://b

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-01 Thread Ioi Lam
ved in > [JDK-8068976](https://bugs.openjdk.java.net/browse/JDK-8068976), so their > declarations should be removed from jvm.h Ioi Lam has updated the pull request incrementally with one additional commit since the last revision: fixed macos build - Changes: - all: http

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-01 Thread Ioi Lam
On Mon, 1 Feb 2021 20:29:10 GMT, Gerard Ziemski wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fixed macos build > > src/java.base/share/native/libjava/check_version.c line 33: >

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-02 Thread Ioi Lam
On Tue, 2 Feb 2021 15:59:47 GMT, Gerard Ziemski wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fixed macos build > > Marked as reviewed by gziemski (Committer). Thanks @gerard-ziemski

Integrated: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX

2021-02-02 Thread Ioi Lam
On Mon, 1 Feb 2021 18:40:54 GMT, Ioi Lam wrote: > - JVM_GetInterfaceVersion() was used by "HotSpot Express" (HSX) which allowed > the same JDK library to use different version of HotSpot. However, HSX is no > longer supported so this API should be removed. > - Implement

Re: RFR: 8260193: Remove JVM_GetInterfaceVersion() and JVM_DTraceXXX [v2]

2021-02-02 Thread Ioi Lam
On Tue, 2 Feb 2021 16:00:43 GMT, Gerard Ziemski wrote: >> I am not sure if jni_utils.c is the right file (it defines the `JNU_XXX` >> functions that are used by other shared libraries). >> >> There are other .c files that have trivial `DEF_JNI_OnLoad` functions (e.g., >> java.base/share/native

Re: RFR: 8223188: Consider rewriting in C++ or moving to ".c" files

2021-02-08 Thread Ioi Lam
On Mon, 8 Feb 2021 22:26:22 GMT, Alexey Semenyuk wrote: > Remove needless `#ifdef __cplusplus` from .cpp sources The Bug Synopsis of 'Consider rewriting in C++ or moving to ".c" files' is unclear. I would request that the Bug Synopsis to be changed to "removed unnecessary #ifdef __cplusplus"

Re: RFR: 8223188: Removed unnecessary #ifdef __cplusplus from .cpp sources [v2]

2021-02-09 Thread Ioi Lam
On Tue, 9 Feb 2021 15:46:55 GMT, Alexey Semenyuk wrote: >> Remove needless `#ifdef __cplusplus` from .cpp sources > > Alexey Semenyuk has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous

Re: RFR: 8260934: java/lang/StringBuilder/HugeCapacity.java fails without Compact Strings

2021-02-09 Thread Ioi Lam
On Tue, 9 Feb 2021 09:20:45 GMT, Aleksey Shipilev wrote: >> $ CONF=linux-x86_64-server-fastdebug make run-test >> TEST=java/lang/StringBuilder/HugeCapacity.java >> TEST_VM_OPTS=-XX:-CompactStrings >> >> STDERR: >> java.lang.OutOfMemoryError: Required length exceeds implementation limit >> at

Re: RFR: 8260710: Inline and simplify String*::{coder, value, isLatin1} methods

2021-02-09 Thread Ioi Lam
On Tue, 9 Feb 2021 13:06:36 GMT, Claes Redestad wrote: >> Since Compact Strings implementation, there are simple methods in String and >> StringBuilders: `coder()`, `value()`, `isLatin1()`. They are mostly there to >> capture `COMPACT_STRINGS` flag that would fold to "false" when compact >> st

Re: RFR: 8260934: java/lang/StringBuilder/HugeCapacity.java fails without Compact Strings

2021-02-09 Thread Ioi Lam
On Tue, 9 Feb 2021 17:56:15 GMT, Aleksey Shipilev wrote: > > There's an existing test that explicitly tests both `-XX:+CompactStrings` > > and `-XX:-CompactStrings`. Maybe we can do the same thing? > > No, I don't think we can. In other tests, the conflict between > `-XX:+CompactStrings` and `

Re: RFR: 8260934: java/lang/StringBuilder/HugeCapacity.java fails without Compact Strings [v2]

2021-02-09 Thread Ioi Lam
On Tue, 9 Feb 2021 19:48:07 GMT, Aleksey Shipilev wrote: >> $ CONF=linux-x86_64-server-fastdebug make run-test >> TEST=java/lang/StringBuilder/HugeCapacity.java >> TEST_VM_OPTS=-XX:-CompactStrings >> >> STDERR: >> java.lang.OutOfMemoryError: Required length exceeds implementation limit >> at

Re: RFR: 8263412: ClassFileInstaller can't be used by classes outside of default package

2021-03-11 Thread Ioi Lam
On Thu, 11 Mar 2021 05:47:00 GMT, Igor Ignatyev wrote: > Hi all, > > could you please review the patch which moves `ClassFileInstaller` class to > `jdk.test.lib.helpers` package? > to reduce changes in the tests, `ClassFileInstaller` in the default package > is kept w/ just `main` method that

Re: RFR: 8263549: 8263412 can cause jtreg testlibrary split

2021-03-12 Thread Ioi Lam
On Sat, 13 Mar 2021 04:31:31 GMT, Igor Ignatyev wrote: > Hi all, > > could you please review this dull patch that replaces `ClassFileInstaller` w/ > `jdk.test.lib.helpers.ClassFileInstaller` in all jtreg test descriptions to > ensure we won't get split testlibrary, and removes > `jdk/test/lib

Re: RFR: 8263549: 8263412 can cause jtreg testlibrary split [v3]

2021-03-12 Thread Ioi Lam
On Sat, 13 Mar 2021 06:16:37 GMT, Ioi Lam wrote: > But I don't understand why this error can happen. It seems like jtreg would > allow two test cases to interfere with each other. The root cause seems to be https://bugs.openjdk.java.net/browse/CODETOOLS-7902847 -

Re: RFR: 8263536: Add missing @compile tags to jpackage tests [v2]

2021-03-13 Thread Ioi Lam
On Sat, 13 Mar 2021 00:51:19 GMT, Alexey Semenyuk wrote: >> 8263536: Add missing @compile tags to jpackage tests > > Alexey Semenyuk has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains one commit: > > 8263536: Add missing @compile ta

Re: RFR: 8263536: Add missing @compile tags to jpackage tests [v2]

2021-03-15 Thread Ioi Lam
On Mon, 15 Mar 2021 19:41:06 GMT, Alexey Semenyuk wrote: >> test/jdk/tools/jpackage/windows/WinDirChooserTest.java line 44: >> >>> 42: * @requires (os.family == "windows") >>> 43: * @modules jdk.jpackage/jdk.jpackage.internal >>> 44: * @compile WinDirChooserTest.java >> >> Changes like this

Re: RFR: 8263536: Add missing @compile tags to jpackage tests [v2]

2021-03-15 Thread Ioi Lam
On Mon, 15 Mar 2021 19:55:03 GMT, Ioi Lam wrote: >> It is better to have `@compile` everywhere for consistency. >> >> The proper way to run tests is by passing test class name as an argument for >> test runner, test class should not have `main()` and should have `@

Re: RFR: 8263536: Add @compile tags to jpackage tests [v2]

2021-03-16 Thread Ioi Lam
On Tue, 16 Mar 2021 16:18:48 GMT, Alexey Semenyuk wrote: > > @compile will always run javac every time you run the test > > Didn't know that. Thank you for explanation. Would this be OK to leave > > `@compile` tags? Since you're making changes, I think we should do it correctly. We should use

Re: RFR: 8263536: Add @build tags to jpackage tests [v3]

2021-03-16 Thread Ioi Lam
On Tue, 16 Mar 2021 17:50:26 GMT, Alexey Semenyuk wrote: >> 8263536: Add @build tags to jpackage tests > > Alexey Semenyuk has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Ioi Lam
On Wed, 17 Mar 2021 15:18:41 GMT, Roger Riggs wrote: >> test/jdk/java/lang/ProcessBuilder/Basic.java line 2183: >> >>> 2181: latch.await(); >>> 2182: Thread.sleep(100L); // Wait for child >>> initialization to settle >>> 2183: >> >> Hi Roger, >> Shouldn't t

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Ioi Lam
On Wed, 17 Mar 2021 16:40:47 GMT, Roger Riggs wrote: >> The failures happened in tiers 6 and 8. The system may be overloaded so even >> 100ms may not be enough for the child process to start sleeping. From the >> error log, the child process tried to spawn a thread (probably one of those >> us

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Ioi Lam
On Wed, 17 Mar 2021 17:30:25 GMT, Thomas Stuefe wrote: >> Arbitrary time out has been a reliable source of intermittent failures. >> >> Since we have spent a lot of time analyzing this failure, I think it's >> worthwhile to fix it properly, which doesn't seem that complicated. That's >> better

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test

2021-03-17 Thread Ioi Lam
On Wed, 17 Mar 2021 19:13:24 GMT, Roger Riggs wrote: >> Hmm. That's strange. >> >> One issue I don't understand with this test, should the parent not read from >> both streams simultaneously, because if the child is sending output to the >> stream the parent is not reading from the child may

Re: RFR: 8263729: [test] Extend time to wait before destroying child in ProcssBuilder Basic test [v3]

2021-03-19 Thread Ioi Lam
On Fri, 19 Mar 2021 17:17:02 GMT, Roger Riggs wrote: >> Intermittent failures on Windows in a test of destroying the child warrant >> extending the time the parent waits after starting the child before >> destroying the child. > > Roger Riggs has updated the pull request incrementally with one

Re: RFR: 8263968: CDS: java/lang/ModuleLayer.EMPTY_LAYER should be singleton

2021-03-22 Thread Ioi Lam
On Mon, 22 Mar 2021 19:40:19 GMT, Aleksei Voitylov wrote: > With CDS and G1, test api/java_lang/ModuleLayer/Boot.html fails in JDK 16. > > After JDK-8253081 with CDS enabled two instances of empty layer appear in the > runtime. One comes from default initialization of > java/lang/ModuleLayer

Re: RFR: 8166727: javac crashed: [jimage.dll+0x1942] ImageStrings::find+0x28

2021-04-01 Thread Ioi Lam
On Thu, 1 Apr 2021 11:48:21 GMT, Jim Laskey wrote: > We should never close the jimage since java threads can still be running > after a hard exit(). LGTM. - Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3304

Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

2021-04-09 Thread Ioi Lam
On Thu, 8 Apr 2021 17:24:38 GMT, Vladimir Kozlov wrote: >> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related >> to Ahead-of-Time Compiler from JDK: >> >> - `jdk.aot` module — the `jaotc` tool >> - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution

Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

2021-04-09 Thread Ioi Lam
On Fri, 9 Apr 2021 16:54:51 GMT, Ioi Lam wrote: >> Vladimir Kozlov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove exports from Graal module to jdk.aot > > LGTM. Just a small nit. > @iklam >

Re: RFR: 8254598: StringDedupTable should use OopStorage

2021-04-27 Thread Ioi Lam
On Tue, 27 Apr 2021 22:47:06 GMT, Coleen Phillimore wrote: >> Please review this change to the String Deduplication facility. It is being >> changed to use OopStorage to hold weak references to relevant objects, >> rather than bespoke data structures requiring dedicated processing phases by >> s

Re: RFR: 8254598: StringDedupTable should use OopStorage

2021-04-27 Thread Ioi Lam
On Fri, 23 Apr 2021 19:48:47 GMT, Kim Barrett wrote: > Please review this change to the String Deduplication facility. It is being > changed to use OopStorage to hold weak references to relevant objects, > rather than bespoke data structures requiring dedicated processing phases by > supporting

Re: RFR: 8254598: StringDedupTable should use OopStorage

2021-04-27 Thread Ioi Lam
On Wed, 28 Apr 2021 00:44:19 GMT, Ioi Lam wrote: >> Please review this change to the String Deduplication facility. It is being >> changed to use OopStorage to hold weak references to relevant objects, >> rather than bespoke data structures requiring dedicated processing phase

Re: RFR: 8254598: StringDedupTable should use OopStorage

2021-05-02 Thread Ioi Lam
On Fri, 23 Apr 2021 19:48:47 GMT, Kim Barrett wrote: > Please review this change to the String Deduplication facility. It is being > changed to use OopStorage to hold weak references to relevant objects, > rather than bespoke data structures requiring dedicated processing phases by > supporting

Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v2]

2021-05-05 Thread Ioi Lam
On Wed, 5 May 2021 16:34:20 GMT, Yumin Qi wrote: >> Hi, Please review >> When using jcmd to dump shared archive, if the archive name exists, it >> will be deleted first. If exception happens during dumping process, there is >> no new archive created. This PR changes to first dump the archiv

Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v2]

2021-05-05 Thread Ioi Lam
On Wed, 5 May 2021 18:42:20 GMT, Yumin Qi wrote: >> src/java.base/share/classes/jdk/internal/misc/CDS.java line 284: >> >>> 282: >>> 283: String tempFileName = getArchiveFileName(archiveFileName); >>> 284: File tempArchiveFile = new File(tempFileName); >> >> I think the logic i

Re: RFR: 8265465: jcmd VM.cds should keep already dumped archive when exceptions happens [v5]

2021-05-06 Thread Ioi Lam
On Thu, 6 May 2021 21:37:35 GMT, Yumin Qi wrote: >> Hi, Please review >> When using jcmd to dump shared archive, if the archive name exists, it >> will be deleted first. If exception happens during dumping process, there is >> no new archive created. This PR changes to first dump the archiv

Re: RFR: 8243287: Removal of Unsafe::defineAnonymousClass [v3]

2021-05-11 Thread Ioi Lam
On Tue, 11 May 2021 14:13:49 GMT, Harold Seigel wrote: >> Please review this large change to remove Unsafe::defineAnonymousClass(). >> The change removes dAC relevant code and changes a lot of tests. Many of >> the changed tests need renaming. I hope to do this in a follow up RFE. >> Some

RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2

2021-05-11 Thread Ioi Lam
This bug was discovered during the development of [JDK-6824466](https://bugs.openjdk.java.net/browse/JDK-6824466): when CDS is enabled, if `BootLoader::loadClassOrNull` is called before `initPhase2`, it would trigger the initialization of the archived module graph in the wrong order. Because of

Re: RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2

2021-05-12 Thread Ioi Lam
On Wed, 12 May 2021 05:51:14 GMT, Ioi Lam wrote: > This bug was discovered during the development of > [JDK-6824466](https://bugs.openjdk.java.net/browse/JDK-6824466): when CDS is > enabled, if `BootLoader::loadClassOrNull` is called before `initPhase2`, it > would trigger the in

Re: RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2 [v2]

2021-05-12 Thread Ioi Lam
or the 3 built-in loaders > (boot/platform/app). > > Testing: tiers1-4 in progress. I also verified that the bug reported by Mandy > is no longer reproducible after I applied this patch on her branch. Ioi Lam has updated the pull request incrementally with one additional commit

Re: RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2 [v3]

2021-05-12 Thread Ioi Lam
or the 3 built-in loaders > (boot/platform/app). > > Testing: tiers1-4 in progress. I also verified that the bug reported by Mandy > is no longer reproducible after I applied this patch on her branch. Ioi Lam has updated the pull request incrementally with one additional commit sinc

Re: RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2 [v2]

2021-05-12 Thread Ioi Lam
On Wed, 12 May 2021 18:46:20 GMT, Alan Bateman wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> @AlanBateman comments > > src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java

Re: RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2 [v4]

2021-05-13 Thread Ioi Lam
or the 3 built-in loaders > (boot/platform/app). > > Testing: tiers1-4 in progress. I also verified that the bug reported by Mandy > is no longer reproducible after I applied this patch on her branch. Ioi Lam has updated the pull request with a new target base due to a merge or a rebas

Re: RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2 [v3]

2021-05-13 Thread Ioi Lam
On Thu, 13 May 2021 06:19:47 GMT, Alan Bateman wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> cleaned up ClassLoaders.setArchivedServicesCatalog > > Marked as reviewed by alanb (Revie

Integrated: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2

2021-05-13 Thread Ioi Lam
On Wed, 12 May 2021 05:51:14 GMT, Ioi Lam wrote: > This bug was discovered during the development of > [JDK-6824466](https://bugs.openjdk.java.net/browse/JDK-6824466): when CDS is > enabled, if `BootLoader::loadClassOrNull` is called before `initPhase2`, it > would trigger the in

  1   2   3   >