On Thu, 22 Apr 2021 06:10:17 GMT, Thomas Stuefe <stu...@openjdk.org> 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 11 additional commits 
>> since the last revision:
>> 
>>  - Merge branch 'master' into 8265696-move-cds-sources
>>  - fixed merge
>>  - Merge branch 'master' into 8265696-move-cds-sources
>>  - Merge branch 'master' into 8265696-move-cds-sources
>>  - Merge branch 'master' into 8265696-move-cds-sources
>>  - exclude all files under shared/cds if CDS is disabled; 
>> compactHashtable.cpp cannot be excluded since a bit of it is used even when 
>> CDS is disabled
>>  - fixed include guards -> #ifndef SHARE_CDS_xxxxx
>>  - fixed copyright
>>  - moved more files
>>  - fixed include lines
>>  - ... and 1 more: 
>> https://git.openjdk.java.net/jdk/compare/f8227451...8a9e7bdf
>
> Hi @iklam,
> 
> this is a very welcome change!
> 
> Nothing much to add to what David wrote (include guards need renaming). 
> 
> Apart from that, I was surprised that no gtests needed to be adapted, but 
> seems cds has no gtests?
> 
> I tested building without cds, works fine.
> 
> Thanks for doing this!
> 
> If you fix the include guards, this is fine by me.
> 
> ..Thomas

Thanks @tstuefe @erikj79 @dholmes-ora for the review.
The latest merged code passes Mach5 build tiers 1-5.
I also manually tested the minimal VM build to verify the exclusion of CDS 
object files.

-------------

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

Reply via email to