Github user markt-asf commented on the issue:
https://github.com/apache/tomcat/pull/108
I have applied the optimisation commit (thanks) but not the ordering
changes.
The patch changes the order for undeploy without changing the order for
version mapping. This will lead to unexpected behaviour.
The current simple `String` based ordering was selected as a trade off
between performance and usability. The type of ordering proposed in this patch
was considered too expensive to incur on every request when multiple versions
are deployed. To consider such a change, we'd need to see evidence of the
performance impact (including GC) on the mapping process of switching to this
ordering mechanism. Be aware that mapping code is highly optimised for `String`.
Reviewing the proposed patch I see several things that would need to be
fixed in additional to the more architectural issue described above:
- imports from sun.* are not allowed
- the code is Java 8 specific - any fix needs to be available to
back-ported to 7.0.x will has to run on Java 6
- the patch does not compile
- `Pattern` instances should be pre-compiled
Finally, the patch only considers purely numeric versions. Many projects
include a test string (e.g. RELEASE) in the version number. It would be nice to
handle these as well.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
