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