Eden space summury in G1GC log might report values with decimal separators

[Stephan202]

I recently had to parse some GC logs produced with JDK 1.7.0_12-ea-b08. Upon loading the file in GCViewer, a bunch of NumberFormatExceptions were reported. I tracked this down to the fact that the following format is currently not supported:

[Eden: 118.0M(118.0M)->0.0B(112.0M) Survivors: 10.0M->16.0M Heap: 548.6M(640.0M)->440.6M(640.0M)]

After some debugging, I noticed that DataReaderSun1_6_0G1#parseDetails calls AbstractDataReaderSun#setMemoryExtended which attempts to parse the values as integers using NumberParser#parseInt. Clearly that fails due to the presence of decimal separators. Also, in AbstractDataReaderSun#setMemoryExtended the for-loop that determines the length of the post-memory number (line 140) assumes that all characters are digits (so not a dot).

It'd be great if GCViewer would support this!

(Locally I added an ugly hack to NumberParser#parseInt in order work around these things (that was faster than generating a new GC log ;)), but these changes are not suitable for sharing. (If you really want me to I can paste them here, though...).)

Semi-relatedly, I noticed that only one of the six methods in NumberParser is used. It might be worthwhile to determine how much of a speedup the NumberParser provides over Java's built-in API. If the difference is not very significant, I'd drop that class entirely and simply fix this issue with some calls to String#substring and Double#parseDouble instead. (And a small change to AbstractDataReaderSun#getMemoryInKiloByte.) If you agree with that approach I'd be willing to make the changes and open a pull request.

[chewiebug]

Hi Stephan

Looks like a new format of the detailed memory output in the latest jdk,
thank you for reporting it!

According to my tests NumberParser is about three times faster than
Integer.parseInt(), so I'd like to keep it. String#substring() seems to
have some performance impact in java 7, but hardly in java 6 - can't be
helped, I suppose. I suggest fixing
AbstractDataReaderSun#setMemoryExtended() (which is only used by the G1
parser) to use Double.parseDouble() including the small change to
|AbstractDataReaderSun#getMemoryInKiloByte|().

If you'd like to make these changes I'll happily wait for the pull request.

Regards, Jörg

[Stephan202]

Hi Jörg,

Sorry for not getting back to you earlier. If you're not in too much of a hurry, I'd be happy to open a pull request in one of the coming days (quite busy at work :)).

[chewiebug]

Hi Stephan

I would like to release about by the end of this month. So "in the
coming days" sounds very good to me :-).

Regards, Jörg

[chewiebug]

Hi Stephan

I have just tested with the latest JDK (7u15) and I don't find decimal separators in the memory output of G1 there. Seeing your jdk version contains a "ea" (seems to be "early access") I wonder if this format is ever going to make it into the productive releases. Do you have any idea?

Regards, Jörg

[Stephan202]

Good catch. Unfortunately I don't know whether this was a fluke or a feature that just hasn't been released yet. The page listing Developer Preview Releases hasn't been updated since January 3 (at the time of writing), so that isn't much help either.

I suggest we hold off on this merge until the format has been cleared up

[chewiebug]

Hi Stephan

Since the decimal separators haven't made it into JDK 7u17, I'll close this issue and the pull request. If in a later release we see this change, I'll know where to go look for the fix.

Thank you anyway for your time!

Regards, Jörg

[ryangardner]

I think this is a future feature. If you look at the output of "java -version" you will see that the hotspot version in the developer preview is a later version than the hotspot in the latest release. With all of the security updates the version numbers have gotten screwed up.