JDK-8241837 : Cleanup stringStream usage in ObjectSynchronizer
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-03-30
  • Updated: 2020-04-27
  • Resolved: 2020-04-01
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 15
15 b18Fixed
Related Reports
Relates :  
Description
stringStream mallocs on initialization, and there are a few uses in ObjectSynchronizer which is only used in case a guarantee fails which would benefit from being guarded. 

For example the stringStream ss in om_alloc is completely unused, but still accounts for roughly 30% of the instructions retired by that method.


Comments
URL: https://hg.openjdk.java.net/jdk/jdk/rev/45e750f0d1c8 User: redestad Date: 2020-04-01 12:34:09 +0000
01-04-2020

That looks better, yes, thanks. :-)
30-03-2020

It would better like this: + if (s->is_busy()) { + stringStream ss; + fatal("must be !is_busy: %s", s->is_busy_to_string(&ss)); + } We've been trying to get rid of 'guarantee(false, ...' calls...
30-03-2020

Same ss-for-guarantee pattern is still there in om_flush / om_release, and I'd like to get rid of it since the stringStream will malloc memory even when the guarantee does not trigger. While not a major performance issue here, I think having mallocs for error management in these sensitive methods might be problematic for other reasons. So while gritty, I'm currently thinking of suggesting this: - stringStream ss; - guarantee(!s->is_busy(), "must be !is_busy: %s", s->is_busy_to_string(&ss)); + if (s->is_busy()) { + stringStream ss; + guarantee(false, "must be !is_busy: %s", s->is_busy_to_string(&ss)); + }
30-03-2020

The "stringStream ss" in ObjectSynchronizer::om_alloc() was added by this fix: $ hg log -r 55345 changeset: 55345:492b644bb9c2 user: dcubed date: Wed Jun 12 10:52:45 2019 -0400 summary: 8225453: is_busy diagnostics and other baseline cleanups from Async Monitor Deflation project The "stringStream ss" was necessary for this guarantee() call in the initial webrev: guarantee(!take->is_busy(), "must be !is_busy: %s", take->is_busy_to_string(&ss)); During the code review for 8225453, that guarantee() was removed in favor of an assert() in the Recyle() function. The stringStream ss" became unused at that point.
30-03-2020