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

Bug 369411

Summary:	[xtext][performance] improve performance of IQualifiedNameConverter.DefaultImpl
Product:	[Modeling] TMF	Reporter:	Knut Wannheden <knut.wannheden>
Component:	Xtext	Assignee:	Project Inbox <tmf.xtext-inbox>
Status:	CLOSED FIXED	QA Contact:
Severity:	enhancement
Priority:	P3	CC:	karsten.thoms, sebastian.zarnekow
Version:	2.2.1	Flags:	sebastian.zarnekow: juno+
Target Milestone:	M6
Hardware:	PC
OS:	Mac OS X - Carbon (unsup.)
Whiteboard:

Description Knut Wannheden

2012-01-23 11:07:59 EST

The default implementation of IQualifiedNameConverter declares a getDelimiter() method which subclasses can override if they want a delimiter other than '.'. The implementation further uses String#split(String) in toQualifiedName(String) to split a string into its segments. But since the delimiter should not be interpreted as a regular expression the implementation uses Pattern#quote().

Using String#split(String) is very inefficient and not necessary since the delimiter is never interpreted as a regular expression. It would be much better to use Guava's Splitter class. It seems to be around twice as fast.

Comment 1 Knut Wannheden

2012-01-23 11:27:20 EST

By "inlining" Guava's Splitter implementation into a for loop it is possible to shave off another 25%. But it may not be worth the added complexity. Measurements for invoking #toQualifiedName() 50'000'000 times:

- current implementation: 34500ms
- Guava's Splitter: 15800ms
- inlined Splitter: 11950ms

Comment 2 Sebastian Zarnekow

2012-01-23 11:31:35 EST

I think inlining the splitting is fine. Could you please apply the changes?

Comment 3 Sebastian Zarnekow

2012-01-23 11:39:23 EST

Btw: there are other possible hotspots:

org.eclipse.xtext.linking.lazy.LazyURIEncoder
org.eclipse.xtext.conversion.impl.QualifiedNameValueConverter
org.eclipse.xtext.common.types.access.impl.ClasspathTypeProvider
org.eclipse.xtext.common.types.access.impl.IndexedJvmTypeAccess
org.eclipse.xtext.xbase.conversion.XbaseQualifiedNameValueConverter

We should probably use org.eclipse.xtext.util.Strings.split(String, String) and improve on that one if necessary (e.g. return an Collection.unmodifiableList and provide JavaDoc ;-)

Comment 4 Knut Wannheden

2012-01-23 15:04:02 EST

I will take a look at these other services. In our product I actually have this optimization also implemented for LazyURIEncoder, as that was found to be a hotspot.

Comment 5 Knut Wannheden

2012-01-24 04:45:36 EST

Pushed to master: http://git.eclipse.org/c/tmf/org.eclipse.xtext.git/commit/?id=276d4c07954df20a341bc5dab944373cf8b398e9

After some more performance testing I found the implementation of Strings#split(String, String) to be very good as-is. I added some Javadoc and an additional Strings#split(String, char) for single character delimiters.

I refactored all of the mentioned services to use either of these methods. I also refactored some additional services:

- EcoreUtil2#getEReferenceFromExternalForm()
- XtextFragmentProvider#getEObject()
- FQNPrefixMatcher#isCandidateMatchingPrefix()
- StaticQualifierPrefixMatcher#isCandidateMatchingPrefix()
- JdtTypesProposalProvider#searchAndCreateProposals()

Comment 6 Sebastian Zarnekow

2012-01-24 05:01:24 EST

Very nice, thanks!

Comment 7 Sebastian Zarnekow

2012-02-17 09:22:17 EST

Turns out that our Strings.split behaves differently than String.split. The latter discards trailing empty strings from the resulting array where we keep them. This broke the StaticQualifierValueConverter in Xbase. We have to double check the places where we replaced the String.split with Strings.split.

Comment 8 Sebastian Zarnekow

2012-02-17 09:27:56 EST

I'll change the semantics of our Strings.split to be similar to String.split

Comment 9 Sebastian Zarnekow

2012-02-17 10:00:39 EST

Pushed to master. Please mind the changes in 
http://git.eclipse.org/c/tmf/org.eclipse.xtext.git/commit/?id=c0882866d2dfd9079058e25674e111e8cbddbd40

Comment 10 Sebastian Zarnekow

2012-02-17 11:36:03 EST

Marked as resolved. Please reopen if the changes cause any trouble

Comment 11 Sebastian Zarnekow

2012-04-12 10:35:22 EDT

*** Bug 376605 has been marked as a duplicate of this bug. ***

Comment 12 Eclipse Webmaster

2017-10-31 11:26:50 EDT

Requested via bug 522520.

-M.