On Thu, May 14, 2020 at 12:20 AM Mark Thomas <ma...@apache.org> wrote:

> On 13/05/2020 19:23, Rémy Maucherat wrote:
> > On Mon, May 4, 2020 at 4:43 PM <ma...@apache.org
> > <mailto:ma...@apache.org>> wrote:
> >
> >     This is an automated email from the ASF dual-hosted git repository.
> >
> >     markt pushed a commit to branch master
> >     in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >     commit 1719b71374d57d59bdcd99537bf13348cdaf87c7
> >     Author: KangZhiDong <world...@gmail.com <mailto:world...@gmail.com>>
> >     AuthorDate: Sat Apr 25 01:30:47 2020 +0800
> >
> >         Avoid waste of resources due to reconstruction of objects
> >     ---
> >      .../apache/catalina/core/ApplicationContext.java   |  2 +-
> >      .../apache/catalina/ha/tcp/ReplicationValve.java   |  2 +-
> >      .../catalina/session/PersistentManagerBase.java    |  2 +-
> >      .../catalina/valves/rewrite/RewriteCond.java       |  2 +-
> >      .../catalina/valves/rewrite/RewriteRule.java       |  4 ++--
> >      .../catalina/valves/rewrite/RewriteValve.java      |  2 +-
> >      java/org/apache/juli/ClassLoaderLogManager.java    |  2 +-
> >      test/org/apache/catalina/valves/Benchmarks.java    | 26
> >     +++++++++++-----------
> >      8 files changed, 21 insertions(+), 21 deletions(-)
> >
> >
> > https://bz.apache.org/bugzilla/show_bug.cgi?id=64432
> > Ok, so this looked like very fishy savings. It turns out each object
> > instance may want to have its own thread local. IMO that's the case for
> > nearly all the classes above, and either way it's probably not a good
> > idea to take any chances.
> > I think this should be reverted.
>
> I went through each of these in turn and checked that a static
> ThreadLocal was safe to use. What do you think I missed?
>

I used the testcases and verified the patch from the BZ:
https://bz.apache.org/bugzilla/attachment.cgi?id=37241
The static thread local would mean all rules are sharing the pattern of the
first rule.

So overall: this optimization using static thread locals sounds dangerous
as it could lead to hard to find bugs later on as soon as there are two or
more instances of an object. So I'd prefer passing on it, even if it would
be acceptable in some cases.

Rémy


>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

Reply via email to