commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adrian Crum <>
Subject Re: [collections] Revert a performance related fix in 4.1
Date Sat, 24 Jan 2015 18:43:35 GMT
Slightly off-topic but somewhat related...

I saw a recent commit where a "performance improvement" went something 
like this:

StringBuilder sb = new StringBuilder();

was replaced with

StringBuilder sb = new StringBuilder("foo");

The change reduced the code by one line, but there was no "performance 
improvement" - because the StringBuilder constructor calls append().

So, I agree that suggested performance improvements should be met with a 
great deal of skepticism and they should be closely scrutinized.

Adrian Crum
Sandglass Software

On 1/24/2015 10:21 AM, Thomas Neidhart wrote:
> Hi,
> from time to time some researchers trying to find performance bugs in
> open-source software create issues for collections.
> One of the easy targets is the Collection#retainAll(Collection) method
> as the default implementation in AbstractCollection calls contains on
> the provided collection.
> Now, in the worst-case this may lead to a runtime complexity of O(n^2),
> i.e. when the collection has a contains implementation with O(n) complexity.
> The proposed solution is usually to create an intermediate set and use
> it to speed up the contains calls.
> My position was always that a given Collection class shall not overwrite
> this method trying to optimize its runtime behavior for any possible
> input by creating such intermediate data structures as this will just
> increase the space complexity (in many cases unnecessarily). Instead,
> the runtime complexity was documented (one can even question this as the
> Collection framework should be well-known by java users).
> It looks like that at 2 occasions these "performance bugs" got fixed:
>   *
>   *
> and I would like to revert these fixes as they are wrong imho and just
> create a precedent for further tickets.
> Does anybody challenge my rationale behind this or can I go ahead with it?
> Thomas
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message