Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make test for unique stacks more robust #1811

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

schmelter-sap
Copy link
Member

This changes the way the stack traces are routed to the test the uniques stacks test, since it produces a big output. We now let the profiled VM print the stacks to stderr and not use the output of jcmd, since the latter is buffered in the profiled VM first and there are hard limits on the maximum size of that buffer,.

fixes #1810

@SapMachine
Copy link
Member

Hello @schmelter-sap, this pull request fulfills all formal requirements.

Copy link
Member

@RealCLanger RealCLanger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes sense, I guess. Let's see whether all tests are green.

OutputAnalyzer oa = callJcmd(p, "MallocTrace.dump", "-percentage=100");
// The output can be large, so dump it directly, since the output of
// jcmd is temporary hold in the JVM and there is a limit of 100 MB.
final String[] stacks = new String[1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you are using a String[] here instead of a single String object?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot modify a local variable in an anonymous inner class. So the array trick is the common way.

Copy link
Member

@RealCLanger RealCLanger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. We should port this fix to upstream, too, I guess?

@schmelter-sap
Copy link
Member Author

OK. We should port this fix to upstream, too, I guess?

If this really fixes the problem in the tests.

@RealCLanger
Copy link
Member

OK. We should port this fix to upstream, too, I guess?

If this really fixes the problem in the tests.

It should - it looks greenish now. And the error happened on all our ppc64le nodes before.

@RealCLanger RealCLanger merged commit 08e70cc into SAP:sapmachine17 Sep 9, 2024
95 of 96 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants