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.

Sign up for GitHub

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

Jump to bottom

improve ClasspathUtilCore.getPath performance eclipse-pde#13 #14

Closed
wants to merge 1 commit into from
Closed

improve ClasspathUtilCore.getPath performance eclipse-pde#13 #14

wants to merge 1 commit into from

Conversation

jukzi
Copy link
Contributor

@jukzi jukzi commented Apr 6, 2022

isAbsolute() is cheap, exists() is I/O. fail fast!

@EcljpseB0T
improve ClasspathUtilCore.getPath performance #13
9fe6f88 
isAbsolute() is cheap, exists() is I/O. fail fast!
@jukzi jukzi mentioned this pull request Apr 6, 2022
Closed
@mickaelistria mickaelistria added performance Performance issues enhancement New feature or request labels Apr 6, 2022
@mickaelistria mickaelistria linked an issue Apr 6, 2022 that may be closed by this pull request
improve org.eclipse.pde.internal.core.ClasspathUtilCore.getPath(IPluginModelBase, String) performance #13
Closed
@vogella vogella self-requested a review April 6, 2022 14:11
@vogella
Copy link
Contributor

vogella commented Apr 6, 2022

Looks good. Will merge after m1 has been released

@HannesWell
Copy link
Member

Looks good. Will merge after m1 has been released

I can only agree with this.

isAbsolute() is cheap, exists() is I/O. fail fast!

Thank you Jörg. I/O is indeed very often surprisingly slow and I already noticed it multiple times that devlopers are not aware of it (at least the write code like the one you have corrected).
But when you have to do I/O it can also make a big difference how you do it.
For example File.copy() is in my experience magnitudes faster (in my case, it was about 200 times) than transferring a FileInputStream to an FileOutputStream.

@jukzi
Copy link
Contributor Author

jukzi commented Apr 7, 2022

superseeded by
#15

@mickaelistria
Copy link
Contributor

superseeded by #15

So should we close this one?

@ghost
Copy link

ghost commented Apr 8, 2022

So should we close this one?

if you agree to merge #15 instead- yes!

@mickaelistria
Copy link
Contributor

#15 looks good to me to be merged as soon as M1 is complete. So let's close this one.

laeubi referenced this pull request in laeubi/eclipse.pde Jan 28, 2024
@niraj-modi
Bug 579509 - IES423:(ICT) Missing space between two strings (#14)
16b7113 
* Bug 579509 - IES423:(ICT) Missing space between two strings
- Fixed missing space between strings
- Added missing Eclipse Copy right note

Change-Id: Ie3bbd2ff42172d1cd5beb0269900cda1039e07d4
Signed-off-by: Niraj Modi <[email protected]>

* Bug 579509 - IES423:(ICT) Missing space between two strings
- Fixed missing space between strings
- Added missing Eclipse Copy right note

Change-Id: Ie3bbd2ff42172d1cd5beb0269900cda1039e07d4
Signed-off-by: Niraj Modi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vogella vogella vogella approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request performance Performance issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

improve org.eclipse.pde.internal.core.ClasspathUtilCore.getPath(IPluginModelBase, String) performance
5 participants
@jukzi @vogella @HannesWell @mickaelistria @EcljpseB0T