[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8260296: SA's dumpreplaydata fails
From:       Chris Plummer <cjplummer () openjdk ! java ! net>
Date:       2021-01-22 22:27:50
Message-ID: jC2BuNrXY3hPz7CsicbBVpQhxsWOIKZdVFpXPLVOf9g=.e847698c-49e5-4080-bae9-3b257e0c9540 () github ! com
[Download RAW message or body]

On Fri, 22 Jan 2021 14:21:52 GMT, Roland Westrelin <roland@openjdk.org> wrote:

> I noticed that the SA's dumpreplaydata command fails with:
> 
> java.lang.AssertionError: CLHSDB wasn't run successfully: Opening core file, please \
> wait... hsdb> Exception in thread "main" java.lang.InternalError: ciMetadata does \
> not appear to be polymorphic 
> with a simple test program. This happens because the SA can't find the
> vtable symbol for ciMetadata (build produced by gcc 9.2.1). AFAIU,
> there's nothing in our build system that hides that symbol. I had to
> move one method's definition from the header file to the cpp file for
> the symbol to be visible again.
> 
> We have a test that checks dumpreplaydata but it doesn't catch that
> problem. The test produces a replay file from a core file with the SA
> by running a simple test with -Xcomp and CICrash=1. So the replay data
> has very little or no profile data (which is what causes the problem
> above). I propose running a slightly more complicated test method and
> crashing after the method has had time to run for long enough to
> collect profile data.
> 
> The other shortcoming of the test is that it doesn't look at the
> content of the replay file. It only warns if they differ. The replay
> file produced by the VM and the one produced by the SA should be
> identical (except for comment lines). So I propose we check that.
> 
> Finally, I can't run that test on my system because core files are
> handled by systemd (I'm running some recent version of fedora). I
> suppose, the system can be configured differently but having the test
> work out the box is nice. I extended the test case to handle that.
> 
> With the improved test, there are a few differences between the VM and
> SA replay files caused by VM changes that were not mirrored in the
> SA. I fixed those.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ProfileData.java line 54:

> 52: 
> 53:   // Low-level accessors for underlying data
> 54:   long intptrAt(int index) {

How did this ever work if it was returning an int?

test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java line 179:

> 177:         }
> 178:         if (needCoreDump) {
> 179:             String coreFileLocation = getCoreFileLocation(crashOutputString, \
> pid);

Please see the comments I just added to JDK-8259501. This test should be using the \
new core file support in CoreUtils.java. I think this means you don't need any of the \
ProcessTools.java changes.

test/hotspot/jtreg/compiler/ciReplay/SABase.java line 144:

> 142:                     System.out.println("1: " + l1);
> 143:                     System.out.println("2: " + l2);
> 144:                     failure = true;

Don't we want to break out of the loop at this point?

test/hotspot/jtreg/compiler/ciReplay/CiReplayBase.java line 314:

> 312:             // file. It can take a few seconds for the system to
> 313:             // process the just produced core file so we may need to
> 314:             // retry a few times.

If this is really the case, shouldn't all the SA core file tests be running into this \
problem? Perhaps the following in CoreUtils.java is why they don't:  } else if \
(Platform.isLinux()) {  // Check if a crash report tool is installed.
            File corePatternFile = new File(CORE_PATTERN_FILE_NAME);
            try (Scanner scanner = new Scanner(corePatternFile)) {
                while (scanner.hasNextLine()) {
                    String line = scanner.nextLine();
                    line = line.trim();
                    System.out.println(line);
                    if (line.startsWith("|")) {
                        System.out.println(
                            "\nThis system uses a crash report tool ($cat \
                /proc/sys/kernel/core_pattern).\n" +
                            "Core files might not be generated. Please reset \
                /proc/sys/kernel/core_pattern\n" +
                            "to enable core generation. Skipping this test.");
                        throw new SkippedException("This system uses a crash report \
tool");  }
                }
            }
        }

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

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


[prev in list] [next in list] [prev in thread] [next in thread]