tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Mikhulya (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (TAP5-2333) Decrease number of ThreadLocal.get calls
Date Mon, 19 May 2014 18:40:38 GMT

    [ https://issues.apache.org/jira/browse/TAP5-2333?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14002146#comment-14002146

Michael Mikhulya commented on TAP5-2333:

I'm not sure if the gain in request thoughput is worth sacrificing the ability to log detailed
information about what led to an error.

I believe 23% performance gain is rather big to make an option for users. It wouldn't be hard
to enable additional logging in case when errors happens.
We use patched tapestry without 'operation descriptions' for 3 months on several projects
w/o any complains from developers. Seems like it is useless at least for developers in our

> 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
> 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
> 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

View raw message