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 
> commit since the last revision:
> 
>   8253208: Move CDS related code to a separate class

Marked as reviewed by iklam (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/261


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

2020-09-21 Thread Erik Joelsson
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 
> commit since the last revision:
> 
>   8253208: Move CDS related code to a separate class

Build changes look good.

-

PR: https://git.openjdk.java.net/jdk/pull/261


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

2020-09-21 Thread Mandy Chung
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 
> commit since the last revision:
> 
>   8253208: Move CDS related code to a separate class

This patch looks okay.   Please add the javadoc for 
`CDS::getRandomSeedForDumping` and `CDS::defineArchiveModules` for
completeness.

-

PR: https://git.openjdk.java.net/jdk/pull/261


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

2020-09-21 Thread Yumin Qi
On Sun, 20 Sep 2020 06:10:53 GMT, Ioi Lam  wrote:

>> Yumin Qi has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8253208: Move CDS related code to a separate class
>
> src/java.base/share/native/libjava/CDS.c line 49:
> 
>> 47: JNIEXPORT jboolean JNICALL
>> 48: Java_jdk_internal_misc_CDS_isCDSDumpingEnabled(JNIEnv *env, jclass jcls) 
>> {
>> 49: return JVM_IsCDSDumpingEnabled(env);
> 
> Maybe: return JVM_IsCDSDynamicDumpingEnabled(env)

updated with comments.

-

PR: https://git.openjdk.java.net/jdk/pull/261


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

2020-09-21 Thread Yumin Qi
> 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 commit 
since the last revision:

  8253208: Move CDS related code to a separate class

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/261/files
  - new: https://git.openjdk.java.net/jdk/pull/261/files/b9789c8b..c01deec7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=261=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=261=00-01

  Stats: 27 lines in 7 files changed: 0 ins; 0 del; 27 mod
  Patch: https://git.openjdk.java.net/jdk/pull/261.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/261/head:pull/261

PR: https://git.openjdk.java.net/jdk/pull/261