Mailing-List: contact commits-help@tapestry.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@tapestry.apache.org
Date: Mon, 26 May 2014 18:24:02 +0000 (UTC)
From: "Michael Mikhulya (JIRA)" <jira@apache.org>
To: commits@tapestry.apache.org
Message-ID: <JIRA.12715197.1400513124604.18724.1401128642831@arcas>
In-Reply-To: <JIRA.12715197.1400513124604@arcas>
References: <JIRA.12715197.1400513124604@arcas>
Subject: [jira] [Commented] (TAP5-2333) Decrease number of ThreadLocal.get
 calls
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Michael Mikhulya commented on TAP5-2333:
----------------------------------------

Thiago, first version of patch provided part of error information in normal log level and full error information with debug log level.
But with second version of patch I found a way to provide original behavior regarding error information. So nobody need to make a choice between performance and good error information.
See my comment from 22 of May:
{quote}
new patch attached
it doesn't break any functionality, but provides the same performance gain.
{quote}

You need to review only third version of patch.

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