rmaucher commented on issue #144: Variable adds final modifier
URL: https://github.com/apache/tomcat/pull/144#issuecomment-470054183
That's a very useful reminder for me, if a PR is pulled, it *also* needs to
have a decent description as well, otherwise misleading information gets into
https://bz.apache.org/bugzilla/show_bug.cgi?id=63191
--- Comment #11 from Mark Thomas ---
Going back to the original thread dump with the "long" and the "short" stack
traces...
It is possible that the short stack trace is Tomcat trying to send the rest of
the message sent via Async.sendText()
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
Mark Thomas changed:
What|Removed |Added
Severity|normal |enhancement
--- Comment #2 from Mark
y987425112 closed pull request #144: Variable adds final modifier
URL: https://github.com/apache/tomcat/pull/144
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub
y987425112 commented on issue #144: Variable adds final modifier
URL: https://github.com/apache/tomcat/pull/144#issuecomment-470055949
I am very sorry, this is the first time I submit the code, because I am
Chinese, my English is a little poor, there are errors in the description.
Thank
markt-asf commented on issue #144: Variable adds final modifier
URL: https://github.com/apache/tomcat/pull/144#issuecomment-470050541
Potential security vulnerabilities should be reported privately to
secur...@tomcat.apache.org. Not in a PR or any other public forum.
That said,
https://bz.apache.org/bugzilla/show_bug.cgi?id=63236
Andy Wilkinson changed:
What|Removed |Added
CC||awilkin...@pivotal.io
--
You are
https://bz.apache.org/bugzilla/show_bug.cgi?id=63237
Andy Wilkinson changed:
What|Removed |Added
CC||awilkin...@pivotal.io
--
You are
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
Andy Wilkinson changed:
What|Removed |Added
CC||awilkin...@pivotal.io
--
You are
rmaucher commented on issue #143: Apply deduplication to certain loaded and
created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470022913
Can you give a metric of actual memory gains made using this change ? Adding
complexity for very little benefit doesn't seem
rmaucher commented on issue #142: Defer creation of encodingToCharsetCache
URL: https://github.com/apache/tomcat/pull/142#issuecomment-470022349
If there is a lookup with Charset.forName, then would the cache be used at
all ? I don't understand this sort of Windows style "startup time"
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #4 from Mark Thomas ---
Of course. That makes option 2 a non-starter.
The other option isn't that far from the current code. I think it is worth
looking at what it does to memory footprint and start-up time so we can see if
the
https://bz.apache.org/bugzilla/show_bug.cgi?id=63237
Remy Maucherat changed:
What|Removed |Added
Severity|normal |enhancement
--- Comment #1 from Remy
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #3 from Remy Maucherat ---
I think this adds complexity and possible problems for no real benefit. For
example option 2 could have a very big cache (all you need to do is request
random charset values) and could need a lot more
y987425112 opened a new pull request #144: Variable adds final modifier
URL: https://github.com/apache/tomcat/pull/144
If these variables do not add final modifiers, there will be ambiguity and
security risks.
This is an
ChristopherSchultz edited a comment on issue #143: Apply deduplication to
certain loaded and created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470156059
I like the idea, but I think the implementation could be better. For
instance:
1. There is no need to
ChristopherSchultz commented on issue #143: Apply deduplication to certain
loaded and created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470156059
I like the idea, but I think. the implementation could be better. For
instance:
1. There is no need to have
Dear Apache Enthusiast,
(You’re receiving this because you are subscribed to one or more user
mailing lists for an Apache Software Foundation project.)
TL;DR:
* Apache Roadshow DC is in 3 weeks. Register now at
https://apachecon.com/usroadshowdc19/
* Registration for Apache Roadshow Chicago is
philwebb commented on issue #143: Apply deduplication to certain loaded and
created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470145284
Sure. I discovered these by using the "Inspections" feature from
[YourKit](https://www.yourkit.com/) on an [embedded Tomcat
rmaucher commented on issue #143: Apply deduplication to certain loaded and
created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470157903
Ok, I'm not sure it is a very significant saving overall (probably not), but
it's still more than I expected.
Yes, +1for
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 0969485 NIO2 should use SocketTimeoutException
The Buildbot has detected a restored build on builder tomcat-trunk while
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/4115
Buildbot URL: https://ci.apache.org/
Buildslave for this Build: silvanus_ubuntu
Build Reason: The
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 12bfd3d NIO2 should use SocketTimeoutException
https://bz.apache.org/bugzilla/show_bug.cgi?id=63237
--- Comment #2 from Phillip Webb ---
@Remy
> I have not tried to measure the time and resource costs of this parsing
The only evidence I have currently are profiling results that appear to
indicate the SAX parser taking a chunk of time
philwebb commented on issue #143: Apply deduplication to certain loaded and
created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470210052
The other option would be to use `String.intern` for these limited cases.
Either way, I'm happy to rework the PR if you have a
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #5 from Phillip Webb ---
@Mark Thanks for pointing me at those previous issues, I should have thought to
search first!
It seems like I may have misunderstood the reason for the cache existing in the
first place. My original
The Buildbot has detected a new failure on builder tomcat-trunk while building
tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-trunk/builds/4117
Buildbot URL: https://ci.apache.org/
Buildslave for this Build: silvanus_ubuntu
Build Reason: The AnyBranchScheduler
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
The following commit(s) were added to refs/heads/master by this push:
new 800bd8a Improve logging when streams cannot be
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new eed1f6a Improve logging when streams cannot be
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from 800bd8a Improve logging when streams cannot be pruned.
new bb67c85 Update translations for improved log
The Buildbot has detected a new failure on builder tomcat-85-trunk while
building tomcat. Full details are available at:
https://ci.apache.org/builders/tomcat-85-trunk/builds/1687
Buildbot URL: https://ci.apache.org/
Buildslave for this Build: silvanus_ubuntu
Build Reason: The
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #6 from Remy Maucherat ---
I don't see the benefit as there's a clear downside and the complexity
skyrockets. IMO you should focus on the String optimization since it doesn't
have these issues.
On my desktop computer, I measured
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #7 from Phillip Webb ---
> I don't see the benefit as there's a clear downside and the
> complexity skyrockets. IMO you should focus on the String
> optimization since it doesn't have these issues.
I'll continue with the String
ChristopherSchultz commented on issue #143: Apply deduplication to certain
loaded and created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470265906
Good point: String.intern is already implemented, and it's already "weak".
Less code, same result = better.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #8 from Mark Thomas ---
Just to note that the performance issue with unknown charsets wasn't memory
related. It was lock contention. Bug 51400 has the details.
Can you provide some context around why 31ms is significant for the
https://bz.apache.org/bugzilla/show_bug.cgi?id=63237
--- Comment #3 from Remy Maucherat ---
Profilers often exaggerate the impact of some operations, so this has to be
taken with an amount of caution. XML parsing is expensive for sure, but the
descriptors are not too huge and
https://bz.apache.org/bugzilla/show_bug.cgi?id=63237
--- Comment #4 from Phillip Webb ---
> MBeanUtils.createRegistry takes 83ms on my computer, which is not exactly the
> end of the world
That's a really interesting metric and quite a significant amount if we're
talking about using embedded
philwebb commented on issue #142: Defer creation of encodingToCharsetCache
URL: https://github.com/apache/tomcat/pull/142#issuecomment-470317822
I've pushed another update now that I know about
https://bz.apache.org/bugzilla/show_bug.cgi?id=51400
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #10 from Phillip Webb ---
@Mark
I've updated the PR based on your comments. There is now a cache for concurrent
access as well as protection against DoS attacks. I've also tried to add some
tests. The list of common charsets and
To whom it may engage...
This is an automated request, but not an unsolicited one. For
more information please visit http://gump.apache.org/nagged.html,
and/or contact the folk at gene...@gump.apache.org.
Project tomcat-tc7.0.x has an issue affecting its community integration.
This
https://bz.apache.org/bugzilla/show_bug.cgi?id=63210
--- Comment #15 from k...@gameldar.com ---
(In reply to Mark Thomas from comment #14)
> Improved fix applied (fixed versions as before).
I've built and tested with both the replication case and our use case with a
custom DataSource factory and
philwebb commented on issue #142: Defer creation of encodingToCharsetCache
URL: https://github.com/apache/tomcat/pull/142#issuecomment-470280939
I've pushed an update following the discussion on
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
@rmaucher
> If there is a
https://bz.apache.org/bugzilla/show_bug.cgi?id=63235
--- Comment #9 from Phillip Webb ---
> Can you provide some context around why 31ms is significant for the use
> case you are considering? It doesn't seem significant for the typical use
> cases we see.
Spring Boot uses Embedded Tomcat as
philwebb commented on issue #143: Apply deduplication to certain loaded and
created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470381178
I've pushed an update that uses `String.intern` for those two places. With
this change 2192 duplicates are reduced to 300 and
rmannibucau commented on issue #143: Apply deduplication to certain loaded and
created Strings
URL: https://github.com/apache/tomcat/pull/143#issuecomment-470392703
Is generating a java class at build time instead of reading the xml
possible? Then a preregistration in a registry would
45 matches
Mail list logo
