Mark, On 4/27/16 7:41 AM, Mark Thomas wrote: > On 27/04/2016 00:03, Mark Thomas wrote: >> On 25/04/2016 16:42, Romain Manni-Bucau wrote: >>> Hi guys, >>> >>> tomcat uses ConcurrentHashMap in few places and doesn't rely on >>> ConcurrentMap API (ApplicationContext IIRC for instance was the case I >>> encounter). This means if you build tomcat with java 8 and run on java 7 it >>> is broken cause of this new KeyViewSet API used on java 8 (returned type is >>> used for method lookup at runtime). >>> >>> Why would you do it? Not sure but several linux distribution do it. >>> >>> I know we could easily ask all linux distro to build using java 7 but at >>> least fedora/redhat/ubuntu are impacted and tomcat can easily without >>> loosing any feature make it passing for that case using the interface >>> instead of the implementation as field type. >>> >>> Do you think it is possible or would you just move it over linux distro? >> >> I've been through the source code and I have a patch locally ready to >> commit that fixes this in 9.0.x. However... >> >> In some places, this would mean changing the API (usually a protected >> field) of an internal component we might reasonably expect some users to >> have extended. That is generally something we try and avoid in a point >> release unless the change is absolutely necessary (e.g. security) or we >> view the chances of it being used as very, very low. >> >> Therefore, I want to review my local changes and split them into two >> commits. The safe one and the API breaking one. I'm not sure the API >> breaking one is going to be back-ported beyond 8.5.x. >> >> Fundamentally, this is an issue for the distro. The Tomcat docs are >> quite specific (see BUILDING.txt) about which version of Java should be >> used to build Tomcat. If a distro wants to mess with that, they get to >> deal with the consequences. >> >>> If >>> this last one: how to ensure we don't get more regression in the future due >>> to another build process for user binaries? >> >> This is generally something we check during the release. It is mostly a >> manual process although it is automated where building with the wrong >> Java version causes issues for DBCP. > > After rather more commits than I expected (it was easier top review > package by package than all in one go) I think this is fixed.
In reading those commits, my only concern was of performance. I think in all cases (Use of Map versus ConcurrentMap versus ConcurrentHashMap), an invokevirtual call is made rather than an invokestatic, so I'm not sure if it makes too much of a difference. But I seem to recall that making an "interface" call used to be slower than a "class" call, because of the extra level of indirection. These days, I'd expect that the JIT solves all of that and the difference is no longer actually measurable. But since some of these classes are used sometimes many times per request, it did get my spidey-sense tingling. -chris --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
