Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 335279

Summary:

Performance: StateHelperImpl: creating of map/set with initial capacity causes resizing

Product:

[Eclipse Project] Equinox

Reporter:

Radoslav Ivanov <radoslav.ivanov>

Component:

Framework

Assignee:

Lazar Kirchev <l.kirchev>

Status:

RESOLVED FIXED

QA Contact:

Severity:

minor

Priority:

CC:

l.kirchev, tjwatson

Version:

unspecified

Target Milestone:

3.7 M6

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
Patch for collections initialization	tjwatson: iplog+, tjwatson: review+

Description Radoslav Ivanov

2011-01-25 02:50:41 EST

Build Identifier: 3.7

I saw method “org.eclipse.osgi.internal.resolver.StateHelperImpl.getExportedPackageMap”, which creates Map and Set with initial capacity. 
 
For example, 
Map<String, Set<ExportPackageDescription>> result = new HashMap<String, Set<ExportPackageDescription>>(11); 
or 
exports = new HashSet<ExportPackageDescription>(1);
 
IMHO, in most cases, it causes array resizing, which, I saw, reflects to performance analysis. Moreover, the logic suggests adding new elements to array.

Reproducible: Always

Comment 1 Thomas Watson

2011-01-25 08:30:19 EST

Lazar would you be willing to provide a patch?

Comment 2 Lazar Kirchev

2011-01-25 08:34:19 EST

(In reply to comment #1)
> Lazar would you be willing to provide a patch?

Yes, I will check the code and provide a patch.

Comment 3 Lazar Kirchev

2011-01-28 01:59:28 EST

Created attachment 187804 [details]
Patch for collections initialization

Here is a possible patch. I removed the initial capacity from the creation of HashMap and HashSet. I think it is better to leave the default initial capacity since we cannot know in advance approximately how much elements will be added to the collection. This may vary largely. Probably we still will have array copies because the actual size could be much more than the default initial capacity of the collection (which is 16), but how much more? 100? 500? We may have a framework with just a handful of bundles, but may have an enterprise installation with hundreds of bundles. Any suggestions here? 
Probably we could try as a heuristic to take the number of bundles in the State and use as initial capacity for the result the number of bundles multiplied by some factor, representing an average expected number of exported packages per bundle, but even this is hard to predict. Yet, this may cause less array copies.

As for the HashSet, it was initialized with 1 because it is expected that one package will be exported by only one bundle. But this is not always the case - when we have several different versions of a bundle, then all of them could export packages with the same name.

Comment 4 Thomas Watson

2011-01-28 08:51:41 EST

(In reply to comment #3)
Thanks for the analysis Lazar.  I think we should release your patch and not try anything more complicated.

Comment 5 Thomas Watson

2011-02-16 17:21:02 EST

I released the patch, but I question if you will actually see any measurable performance improvements here.  Especially for the case where we have a HashSet containing the packages.  This actually will likely use more memory, for what I think is the common case, when there is only one version of a package available at runtime.