Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

performance: cache URLs of sourceClasspath #990

Merged
merged 1 commit into from
Nov 21, 2016

Conversation

pvojtechovsky
Copy link
Collaborator

This PR improves performance of MainTest.testMain() test on MS Windows 10 to 50% of origin time. From 45s to 22s.

@pvojtechovsky
Copy link
Collaborator Author

It looks like build engine (Travis) is ill...

@monperrus
Copy link
Collaborator

what's the reason, the call to toURI().toURL()?

@pvojtechovsky
Copy link
Collaborator Author

toURI().toURL() is not my code. It was there before. So I do not know. I know that in java 4,5,6 theref was different behavior in file to url conversion. May be author solved that problem...

@monperrus
Copy link
Collaborator

what about removing field String[] sourceClasspath so as to avoid duplicating the state? (and keeping a backward compatible API with on-the-fly translation)

@pvojtechovsky
Copy link
Collaborator Author

removing field String[] sourceClasspath so as to avoid duplicating the state?

good idea. I will try it.

I have noticed that instance of ClassLoader returned by

ClassLoader getInputClassLoader()

is not cached too, Normaly it is not ok to create new class loader fot each java class, but I have not analyzed yet, how it is used in spoon. May be it is intentional here. I measured no performance change with cached class loader.

@tdurieux
Copy link
Collaborator

If I correctly remember getInputClassLoader() is not used by Spoon.

- to make the loaded classes compatible
- to make spoon faster (creation of class loader and classpath URLs for
each runtime class access is slow
@pvojtechovsky
Copy link
Collaborator Author

I have checked how getInputClassLoader() is used. It is used for loading of classes whenever java reflection is used by spoon to gather information about classes.
So I changed the implementation of this PR. It no more caches urlClasspaths, but it caches inputClassLoader instead.

The input classloaded must be cached, because otherwise two classes loaded by two different (not cached) input classloaders are not compatible.

See the new CompilationTest.testSingleClassLoader() for details.
As a side effect of this change the urlClasspath() is called only once during creation of inputClassLoader, so the performance improvement is same like with origin solution of this PR.

The call of setSourceClasspath resets the inputClassLoader cache so the new class loader is created when needed first time.

@monperrus monperrus merged commit 80a8aee into INRIA:master Nov 21, 2016
@monperrus
Copy link
Collaborator

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants