Hi Claes, core libs developers, As promised, we did yesterday our hackathon in the Bulgarian Java User Group and incorporated the feedback that we got from you. Here is our next webrev:
http://dmitryalexandrov.net/~bgjug/5050783/webrev.01/ As you can see: - We got rid of the inner classes altogether in favor of java.util.function.Consumer - The javadoc was updated to follow the standards [1] - In the tests we use an exception that we create rather than relying on one from the JDK that potentially could change We also ran some microbenchmarks to compare the performance of the new functionality with the Apache commons solution mentioned in the bug. Here is the benchmark test: @State(Scope.Benchmark) public class ThrowableStacktraceBenchmark { Throwable chainedThrowable; Throwable simpleThrowable; @Setup public void prepare() { chainedThrowable = createThrowable(64); simpleThrowable = new Throwable("Simple Throwable"); } Throwable createThrowable(int depthLevel) { Throwable t = null; for (int i = 0; i < depthLevel; ++i) { t = new Throwable("My dummy Throwable " + i, t); } return t; } @Benchmark public void benchmarkThrowableGetStackTraceString() { chainedThrowable.getStackTraceString(); } @Benchmark public void benchmarkThrowableGetStackTraceStringSmall() { simpleThrowable.getStackTraceString(); } @Benchmark public void benchmarkThrowablePrintStackTraceToStringWriter() { StringWriter sw = new StringWriter(); chainedThrowable.printStackTrace(new PrintWriter(sw)); sw.toString(); } @Benchmark public void benchmarkThrowablePrintStackTraceToStringWriterSmall() { StringWriter sw = new StringWriter(); simpleThrowable.printStackTrace(new PrintWriter(sw)); sw.toString(); } } And here are the results (test was run with the following options -wi 5 -i 5 -t 1): Benchmark Mode Samples Score Error Units o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowableGetStackTraceString thrpt 50 25351.743 ± 347.326 ops/s o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowableGetStackTraceStringSmall thrpt 50 134990.643 ± 760.225 ops/s o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowablePrintStackTraceToStringWriter thrpt 50 22730.786 ± 167.378 ops/s o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowablePrintStackTraceToStringWriterSmall thrpt 50 115223.025 ± 936.770 ops/s Just in any case we ran this test, actually just the last two methods, with JDK 1.9 ea build 45. We wanted to compare the result of the Apache commons solution with inner classes (as it is now) to the same, but with java.util.function.Consumer. There are the results with the inner classes: o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowablePrintStackTraceToStringWriter thrpt 50 22974.324 ± 391.564 ops/s o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowablePrintStackTraceToStringWriterSmall thrpt 50 114973.452 ± 1051.276 ops/s So you can see that the performance is nearly the same (I guess that the difference is ignorable). Please share with us the feedback about this proposal! Thanks, Ivan [1] http://www.oracle.com/technetwork/articles/java/index-137868.html On Wed, Dec 31, 2014 at 3:32 PM, Claes Redestad <claes.redes...@oracle.com> wrote: > Hi, > > seems like a reasonable convenience method. :-) > > Process stuff: OCA/JSPA signing aside, the patch needs to be submitted in > its > entirety to some part of the OpenJDK infrastructure before it can be > accepted. > Attaching the patch in an e-mail to this mailing list is acceptable for a > small > patch like this when lacking upload privileges to cr.openjdk.java.net. A > sponsor > could help you with the uploading etc. > > This request adds a new public method, which means some internal approval > will be necessary. This is something you'll need help with from an Oracle > sponsor. I could volunteer to initiate the requests internally. > > Code feedback: > > The test relies on the message generated when provoking a division by > zero. I'm not sure this is a good idea nor the place to enforce this > message format with a test. Perhaps just throwing a RuntimeException > with some specified message and test for that? > > May I suggest to also add a test to ensure the method behaves well for an > exception with fillInStackTrace overridden to produce stackless exceptions? > > Just a thought... I noticed PrintWriter, PrintStream and StringBuilder all > inherit from Appendable, so the PrintStreamOrWriter/StackTracePrinter > classes > could be replaced with a single StackTraceAppendable taking an Appendable. > One > static class instead of 1 abstract and 3 concrete static inner classes > could work > out to be a good deal, but there's some odd mechanics in > BufferedWriter/PrintWriter > to use the value of the line.separator property at object creation time > which > would be hard to support without exposing the lineSeparator fields to > Throwable. > Perhaps a future consideration. > > Performance characteristics might change ever so slightly either way, so it > would be nice with some microbenchmarks to quantify. > > /Claes > > > On 2014-12-31 11:51, Ivan St. Ivanov wrote: > >> Hello, >> >> I am Ivan from the Bulgarian Java User Group. As part of our Adopt OpenJDK >> efforts, we organized hackathon beginning of this month in Sofia. There we >> worked on the following JIRA issue: >> >> https://bugs.openjdk.java.net/browse/JDK-5050783 >> >> Unfortunately none of us has author privileges, so the issue is still >> unassigned. But, nevertheless, our JUG has already signed JSPA and some of >> our members (including myself) have signed OCA. And we informed in a >> special email this mailing list about our plans prior to the meeting. >> >> Our hackathon was already documented by our JUG member Mihail Stoynov: >> >> http://mihail.stoynov.com/2014/12/12/building-openjdk- >> and-submitting-a-solution-on-a-feature-request-with-the- >> bulgarian-java-user-group/ >> >> We used the days after the meeting for offline polishing our contribution >> and adding some more unit tests. Here is the result of our work: >> >> http://dmitryalexandrov.net/~bgjug/5050783/webrev.00/ >> >> We would be very happy if you are able to review it, share your feedback >> and guide us through the contribution process as this is our first >> attempt. >> >> Thanks and have a very successful year! >> Ivan >> > >
