[bg-jug] Re: RFR: 5050783: Throwable convenience method: String getStackTraceString()
Yavor Nikolov
nikolov.javor at gmail.com
Fri Jan 16 20:14:33 UTC 2015
Hi,
On that note, I'm thinking loudly about whether or not it'd be appropriate
> to extract common methods in PrintStream and PrintWriter to an interface.
> java.io.Printable?
>
Thinking a bit further in a similar direction: a thinner interface gives
more clarity about the API contract, it's also easier to override,
especially if eligible for lambdas. I speculatively guess that logging
implementations might also benefit from that: I mean simpler API like
printStackTrace(Printable).
Regarding the Appendable idea: one concern has been that println(s) and
append(x + NL) have different behaviour in case of auto-flush-enabled
output. Do we need to bother preserving printStackTrace auto-flush
behaviour? (The Javadoc doesn't specify it, yet in real world there might
have been assumptions that it auto-flushes...).
Best regards,
Yavor
On Thu, Jan 15, 2015 at 3:05 PM, Ivan St. Ivanov <ivan.st.ivanov at gmail.com>
wrote:
> Thanks everybody for the feedback! :)
>
> Claes, we were deliberately trying to avoid the "Apache commons" solution.
> It looks perfectly fine outside of the JDK. But our wild guess was that as
> we have access to the Throwable private methods, we can do something more
> elegant. The fact is (and I shared it with you in our private conversation)
> that we now understood that most elegant is not always the best when it
> comes to the core of the core of Java :)
>
> On that note, I'm thinking loudly about whether or not it'd be
> appropriate to extract common methods in PrintStream and
> PrintWriter to an interface. java.io.Printable?
>
> This would allow getting rid of most inner classes in j.l.Throwable
> along with some duplicated code across the JDK (I've noticed
> at least a few places where there are identical methods which
> differ only in that one takes a PrintStream, the other a PrintWriter).
>
> Do you think that we can experiment with that as part of the effort for
> this issue, or you think it is better to have it as a separate work item
> (possibly done by someone with more experience) and apply it here once it
> is ready?
>
> Regards,
> Ivan
>
>
> On Thu, Jan 15, 2015 at 2:34 PM, David Holmes <david.holmes at oracle.com>
> wrote:
>
>> On 15/01/2015 6:34 AM, Ivan St. Ivanov wrote:
>>
>>> Hi Remi,
>>>
>>> I tried it and, well, the JDK image now does not even compile classes
>>> that
>>> contain lambda expressions:
>>>
>>> javac -cp . ./LambdaMetaFactoryTester.java
>>> Exception in thread "main"
>>> Exception: java.lang.BootstrapMethodError thrown from the
>>> UncaughtExceptionHandler in thread "main"
>>>
>>> With the old Throwable implementation it printed a long stack trace of
>>> exceptions, but at least my sample class was compiled.
>>>
>>> So, I have two questions:
>>> 1) Does anyone dump stack traces in such location, even for debugging
>>> purposes?
>>> 2) If yes, what are the other JDK classes and methods where we should
>>> avoid
>>> using lambdas?
>>>
>>
>> It is hard to be definitive, but the initialization sequence for the JDK
>> classes is extremely fragile. So anything that gets initialized very early
>> in the VM initialization sequence needs to avoid using features that would
>> require that the initialization sequence change. Using -verbose:class shows
>> the order in which classes get loaded.
>>
>> David
>>
>>
>> Thanks,
>>> Ivan
>>>
>>> On Wed, Jan 14, 2015 at 9:44 PM, Remi Forax <forax at univ-mlv.fr> wrote:
>>>
>>> While using a lambda is cool in that context,
>>>> if someone tries to do a printStackTrace in the LambdaMetafactory (for
>>>> debugging purpose by example),
>>>> it will be fun to debug.
>>>>
>>>> cheers,
>>>> Rémi
>>>>
>>>>
>>>> On 01/14/2015 08:25 PM, Ivan St. Ivanov wrote:
>>>>
>>>> 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.benchmarkThrowableGetStackTrac
>>>>> eString
>>>>> thrpt 50 25351.743 ± 347.326 ops/s
>>>>> o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowableGetStackTrac
>>>>> eStringSmall
>>>>> thrpt 50 134990.643 ± 760.225 ops/s
>>>>> o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowablePrintStackTr
>>>>> aceToStringWriter
>>>>> thrpt 50 22730.786 ± 167.378 ops/s
>>>>> o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowablePrintStackTr
>>>>> aceToStringWriterSmall
>>>>> 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.benchmarkThrowablePrintStackTr
>>>>> aceToStringWriter
>>>>> thrpt 50 22974.324 ± 391.564 ops/s
>>>>> o.m.t.ThrowableStacktraceBenchmark.benchmarkThrowablePrintStackTr
>>>>> aceToStringWriterSmall
>>>>> 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.redestad at 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
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>
> --
> You received this message because you are subscribed to the Google Groups
> "Bulgarian Java Users Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to bg-jug+unsubscribe at googlegroups.com.
> To post to this group, send email to bg-jug at googlegroups.com.
> Visit this group at http://groups.google.com/group/bg-jug.
> For more options, visit https://groups.google.com/d/optout.
>
More information about the core-libs-dev
mailing list