[
https://issues.apache.org/jira/browse/TAP5-2333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14008784#comment-14008784
]
Thiago H. de Paula Figueiredo commented on TAP5-2333:
-----------------------------------------------------
Michael, Tapestry prides itself on being a framework which provides very useful
and detailed error pages, so removing the error information isn't acceptable.
For example, from this Howard's blog post:
http://tapestryjava.blogspot.com.br/2013/11/improving-clojure-feedback-stack-traces.html:
"However, this is just the start. Providing truly useful feedback is more than
just putting a patch on exception reporting. It requires attention to detail
throughout the tool. This has been the case for Apache Tapestry, where a
significant amount of the framework is about detecting and reporting errors in
a useful way." A 23% performance improvement is worth nothing if you don't know
what happened when an error occurs.
One option would be to create a configuration symbol to control this and set
the default to continue logging as before, so the default past behavior is kept
and anyone who wants to sacrifice error information for performance is able to
do that with a single configuration change.
> Decrease number of ThreadLocal.get calls
> ----------------------------------------
>
> Key: TAP5-2333
> URL: https://issues.apache.org/jira/browse/TAP5-2333
> Project: Tapestry 5
> Issue Type: Improvement
> Reporter: Michael Mikhulya
> Labels: performance
> Attachments:
> 0001-TAP5-2333-Decrease-number-of-ThreadLocal.get-calls.patch,
> 0002-TAP5-2333-Decrease-number-of-ThreadLocal.get-calls.patch,
> 0003-TAP5-2333-Decrease-number-of-ThreadLocal.get-calls.patch
>
>
> During profiling I found that ThreadLocal.get is a very hot method call.
> Most frequently it is called from PerThreadOperationTracker.
> PerThreadOperationTracker can be replaced with SimpleOperationTracker which I
> introduced in a patch.
> SimpleOperationTracker only prints exception without "operations trace".
> "Operations trace" can be useful during debug. So in my patch
> PerThreadOperationTracker is used in debug mode, but otherwise
> SimpleOperationTracker is used.
> Please check whether this decision is good for most cases.
> Performance gains are very serious.
> Time per request decreased on 11ms (23% of overall time).
> All measurements are done with apache benchmark after warm up phase.
> Currently my patch breaks two tests:
> CoreBehaviorsTests. event_handler_return_types
> MiscTests. operation_tracking_via_annotation
> Both tests are broken because tests depend on 'Operation description' which
> is ignored by SimpleOperationTracker.
> The simplest way to fix tests is to enforce using PerThreadOperationTracker
> for these tests.
> I'm not sure whether 'Operation description' is definitely useful especially
> taking into account that only 2 tests become broken. We use
> SimpleOperationTracker on production for several months already and nobody
> notice that 'Operation descriptions' are absent. In all cases (for us) it was
> enough to see a stack trace of exception in logs.
> See TAP5-2332. There is a huge amount of String.format required to track such
> 'operation descriptions'. By removing calculation of such 'operation
> descriptions' Tapestry can be made much faster.
> If 'Operation descriptions' is required for some cases than we can introduce
> some option to enable/disable this feature.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
