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

improve org.eclipse.pde.internal.core.ClasspathUtilCore.getPath(IPluginModelBase, String) performance #13

Closed
jukzi opened this issue Apr 6, 2022 · 3 comments
Assignees
Labels
performance Performance issues
Milestone

Comments

@jukzi
Copy link
Contributor

jukzi commented Apr 6, 2022

a) During eclipse startup with many external libraries i see some seconds spend in ClasspathUtilCore.getPath
image

There is a simple improvement possible:
instead of
if (file.exists() && file.isAbsolute())

use the inverse order:

if (file.isAbsolute() && file.exists())

since file.exists is an costly IO operation while isAbsolute() is only fast textual evaluation ("starts with slash?").
Thats not much overall but since it's so cheep to improve ... would you please ... "src.zip" is never an absolute filename!

b) A more highlevel problem is that we have a lot projects which all reference the same external libraries (like plugins\org.eclipse.core.runtime_3.24.0.v20210910-0750.jar). Kinda bad that this "exits" checks are done over and over for the same file.

c) the file "org.eclipse.osgi_3.17.100.v20211104-1730.jar\src.zip" will also never exist.... because that "jar" is not an directory but an archive... i guess pde does somewhere know its not a "dir" shaped bundle. - i just do not know where that information is stored.

Stacktrace is for example:

        at org.eclipse.pde.internal.core.ClasspathUtilCore.getPath(ClasspathUtilCore.java:220)
        at org.eclipse.pde.internal.core.ClasspathUtilCore.getSourceAnnotation(ClasspathUtilCore.java:186)
        at org.eclipse.pde.internal.core.PDEClasspathContainer.addExternalPlugin(PDEClasspathContainer.java:80)
        at org.eclipse.pde.internal.core.RequiredPluginsClasspathContainer.addPlugin(RequiredPluginsClasspathContainer.java:352)
        at org.eclipse.pde.internal.core.RequiredPluginsClasspathContainer.addDependency(RequiredPluginsClasspathContainer.java:297)
        at org.eclipse.pde.internal.core.RequiredPluginsClasspathContainer.addDependency(RequiredPluginsClasspathContainer.java:309)
        at org.eclipse.pde.internal.core.RequiredPluginsClasspathContainer.addDependency(RequiredPluginsClasspathContainer.java:280)
        at org.eclipse.pde.internal.core.RequiredPluginsClasspathContainer.computePluginEntries(RequiredPluginsClasspathContainer.java:162)
        at org.eclipse.pde.internal.core.RequiredPluginsClasspathContainer.getClasspathEntries(RequiredPluginsClasspathContainer.java:112)
        at org.eclipse.jdt.internal.core.JavaModelManager.containerPutIfInitializingWithSameEntries(JavaModelManager.java:780)

@mickaelistria
Copy link
Contributor

PR for those changes would be very welcome!

@vik-chand vik-chand added the performance Performance issues label Apr 6, 2022
@jukzi
Copy link
Contributor Author

jukzi commented Apr 6, 2022

added 2 identical commits :-(
rename jukzi to GitHubNewb --force.

Why is here no link to the pull request? (#14) Otherway around there is.

vogella pushed a commit that referenced this issue Apr 8, 2022
Get rid of some File.isFile() I/O where it is impossible that such file
exists anyway:
"file.jar/src.zip"
"directory/absoulteFileName"

In my usecase it reduces getSourceAnnotation() from 3sec to 1.2sec

Signed-off-by: Joerg Kubitz <[email protected]>
@vogella
Copy link
Contributor

vogella commented Apr 19, 2022

AFAICS the change from @jukzi is merged. I close this issue, Jörg, please reopen if you still have pending changes here.

@vogella vogella closed this as completed Apr 19, 2022
@vik-chand vik-chand added this to the 4.24 M2 milestone Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants