RFR: 8264413: Data is written to file header even if its CRC32 was calculated
Locked
 
|
Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments):
$ diff dump.log run.log 17c17 < - base_archive_is_default: 0 --- > - base_archive_is_default: 1 19c19 < - base_archive_name_size: 0 --- > - base_archive_name_size: 113 The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 which results in the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size before calculating CRC32 if needed. Thanks~ Yang ------------- Commit messages: - Data is written to file header even if its CRC32 was calculated Changes: https://git.openjdk.java.net/jdk/pull/3261/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3261&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264413 Stats: 26 lines in 3 files changed: 16 ins; 8 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3261.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3261/head:pull/3261 PR: https://git.openjdk.java.net/jdk/pull/3261 |
|
|
On Tue, 30 Mar 2021 09:25:20 GMT, Yi Yang <[hidden email]> wrote:
> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments): > > $ diff dump.log run.log > 17c17 > < - base_archive_is_default: 0 > --- >> - base_archive_is_default: 1 > 19c19 > < - base_archive_name_size: 0 > --- >> - base_archive_name_size: 113 > > The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 > > which results in the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size before calculating CRC32 if needed. > > Thanks~ > Yang We can freeze(just like Metaspace::freeze) the FileMapInfo after CRC32 was calculated, any subsequent writing actions would be reported as illegal. Do you think this is worth doing? Looking forward to your comments. ------------- PR: https://git.openjdk.java.net/jdk/pull/3261 |
|
|
In reply to this post by Yi Yang
> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments):
> > $ diff dump.log run.log > 17c17 > < - base_archive_is_default: 0 > --- >> - base_archive_is_default: 1 > 19c19 > < - base_archive_name_size: 0 > --- >> - base_archive_name_size: 113 > > The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 > > Now the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size into header **before** calculating CRC32(if needed). > > Thanks~ > Yang Yi Yang 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 the PR. The pull request contains one new commit since the last revision: Data is written to file header even if its CRC32 was calculated ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3261/files - new: https://git.openjdk.java.net/jdk/pull/3261/files/c5a051aa..40b70a2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3261&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3261&range=00-01 Stats: 4 lines in 2 files changed: 1 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3261.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3261/head:pull/3261 PR: https://git.openjdk.java.net/jdk/pull/3261 |
|
|
In reply to this post by Yi Yang
> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments):
> > $ diff dump.log run.log > 17c17 > < - base_archive_is_default: 0 > --- >> - base_archive_is_default: 1 > 19c19 > < - base_archive_name_size: 0 > --- >> - base_archive_name_size: 113 > > The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 > > Now the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size into header **before** calculating CRC32(if needed). > > Thanks~ > Yang Yi Yang 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 the PR. The pull request contains one new commit since the last revision: Data is written to file header even if its CRC32 was calculated ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3261/files - new: https://git.openjdk.java.net/jdk/pull/3261/files/40b70a2a..4502220d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3261&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3261&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3261.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3261/head:pull/3261 PR: https://git.openjdk.java.net/jdk/pull/3261 |
Re: RFR: 8264413: Data is written to file header even if its CRC32 was calculated [v3]
|
|
On Tue, 30 Mar 2021 13:53:34 GMT, Yi Yang <[hidden email]> wrote:
>> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments): >> >> $ diff dump.log run.log >> 17c17 >> < - base_archive_is_default: 0 >> --- >>> - base_archive_is_default: 1 >> 19c19 >> < - base_archive_name_size: 0 >> --- >>> - base_archive_name_size: 113 >> >> The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): >> >> https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 >> >> https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 >> >> Now the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size into header **before** calculating CRC32(if needed). >> >> Thanks~ >> Yang > > Yi Yang 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 the PR. The pull request contains one new commit since the last revision: > > Data is written to file header even if its CRC32 was calculated Looks good. I've tried your patch on linux-x64. It passed all cds tests and also in dynamic archive mode by specifying `-vmoptions:'-Dtest.dynamic.cds.archive=true -XX:+VerifySharedSpaces'` and test group `<open repo>/test/hotspot/jtreg:hotspot_appcds_dynamic` to the jtreg command line. It also passed the first 4 tiers of testing on Oracle supported platforms. I've filed the following RFE for adding a test group for running CDS tests with -XX:+VerifySharedSpaces: https://bugs.openjdk.java.net/browse/JDK-8264472 ------------- Marked as reviewed by ccheung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3261 |
|
|
In reply to this post by Yi Yang
> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments):
> > $ diff dump.log run.log > 17c17 > < - base_archive_is_default: 0 > --- >> - base_archive_is_default: 1 > 19c19 > < - base_archive_name_size: 0 > --- >> - base_archive_name_size: 113 > > The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 > > Now the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size into header **before** calculating CRC32(if needed). > > Thanks~ > Yang Yi Yang has updated the pull request incrementally with one additional commit since the last revision: add new jtreg test with VerifySharedSpaces ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3261/files - new: https://git.openjdk.java.net/jdk/pull/3261/files/4502220d..c4b27fdc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3261&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3261&range=02-03 Stats: 72 lines in 1 file changed: 72 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3261.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3261/head:pull/3261 PR: https://git.openjdk.java.net/jdk/pull/3261 |
|
|
In reply to this post by Calvin Cheung
On Wed, 31 Mar 2021 01:59:35 GMT, Calvin Cheung <[hidden email]> wrote:
> I've tried your patch on linux-x64. It passed all cds tests and also in dynamic archive mode by specifying -vmoptions:'-Dtest.dynamic.cds.archive=true -XX:+VerifySharedSpaces' and test group <open repo>/test/hotspot/jtreg:hotspot_appcds_dynamic to the jtreg command line. It also passed the first 4 tiers of testing on Oracle supported platforms. Besides, I've added a new dynamic cds test with -XX:+VerifySharedSpaces. > I've filed the following RFE for adding a test group for running CDS tests with -XX:+VerifySharedSpaces: https://bugs.openjdk.java.net/browse/JDK-8264472 Looks good! ------------- PR: https://git.openjdk.java.net/jdk/pull/3261 |
Re: RFR: 8264413: Data is written to file header even if its CRC32 was calculated [v4]
|
|
In reply to this post by Yi Yang
On Wed, 31 Mar 2021 03:20:53 GMT, Yi Yang <[hidden email]> wrote:
>> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments): >> >> $ diff dump.log run.log >> 17c17 >> < - base_archive_is_default: 0 >> --- >>> - base_archive_is_default: 1 >> 19c19 >> < - base_archive_name_size: 0 >> --- >>> - base_archive_name_size: 113 >> >> The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): >> >> https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 >> >> https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 >> >> Now the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size into header **before** calculating CRC32(if needed). >> >> Thanks~ >> Yang > > Yi Yang has updated the pull request incrementally with one additional commit since the last revision: > > add new jtreg test with VerifySharedSpaces Marked as reviewed by ccheung (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/3261 |
Re: RFR: 8264413: Data is written to file header even if its CRC32 was calculated [v4]
|
|
In reply to this post by Yi Yang
On Wed, 31 Mar 2021 03:20:53 GMT, Yi Yang <[hidden email]> wrote:
>> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments): >> >> $ diff dump.log run.log >> 17c17 >> < - base_archive_is_default: 0 >> --- >>> - base_archive_is_default: 1 >> 19c19 >> < - base_archive_name_size: 0 >> --- >>> - base_archive_name_size: 113 >> >> The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): >> >> https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 >> >> https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 >> >> Now the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size into header **before** calculating CRC32(if needed). >> >> Thanks~ >> Yang > > Yi Yang has updated the pull request incrementally with one additional commit since the last revision: > > add new jtreg test with VerifySharedSpaces LGTM. ------------- Marked as reviewed by minqi (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3261 |
|
|
In reply to this post by Calvin Cheung
On Wed, 31 Mar 2021 16:00:24 GMT, Calvin Cheung <[hidden email]> wrote:
>> Yi Yang has updated the pull request incrementally with one additional commit since the last revision: >> >> add new jtreg test with VerifySharedSpaces > > Marked as reviewed by ccheung (Reviewer). Thanks @calvinccheung @yminqi for reviews. ------------- PR: https://git.openjdk.java.net/jdk/pull/3261 |
|
|
In reply to this post by Yi Yang
On Tue, 30 Mar 2021 09:25:20 GMT, Yi Yang <[hidden email]> wrote:
> Many tests under `runtime/cds/appcds/dynamicArchive` are crashed when turning on VerifySharedSpaces, VM reports inconsistent crc32 between dumptime and runtime(See detailed content on JBS attachments): > > $ diff dump.log run.log > 17c17 > < - base_archive_is_default: 0 > --- >> - base_archive_is_default: 1 > 19c19 > < - base_archive_name_size: 0 > --- >> - base_archive_name_size: 113 > > The root cause is that even if the CRC32 is calculated(Line 1902), ArchiveBuilder is still writing data to FileMapInfo(Line 1903): > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/archiveBuilder.cpp#L1091-L1094 > > https://github.com/openjdk/jdk/blob/4ea6abfbd1818fa7049bc4f6e46d568e842acb34/src/hotspot/share/memory/filemap.cpp#L1236-L1251 > > Now the expected CRC32(serialized in CDS archive) is different from the calculated ones at runtime. This patch addresses this problem, it writes base_archive_name and size into header **before** calculating CRC32(if needed). > > Thanks~ > Yang This pull request has now been integrated. Changeset: de495df7 Author: Yi Yang <[hidden email]> Committer: Calvin Cheung <[hidden email]> URL: https://git.openjdk.java.net/jdk/commit/de495df7 Stats: 98 lines in 4 files changed: 87 ins; 8 del; 3 mod 8264413: Data is written to file header even if its CRC32 was calculated Reviewed-by: ccheung, minqi ------------- PR: https://git.openjdk.java.net/jdk/pull/3261 |
«
Return to OpenJDK Hotspot Runtime System
|
1 view|%1 views
| Free forum by Nabble | Edit this page |