Bug 174148 - URIUtil.toURI(IPath) and toURI(String) very expensive
Summary: URIUtil.toURI(IPath) and toURI(String) very expensive
Status: CLOSED WONTFIX
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Resources (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P4 minor (vote)
Target Milestone: ---   Edit
Assignee: Platform-Resources-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords: performance
: 130736 174147 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-02-14 06:57 EST by Dani Megert CLA
Modified: 2022-02-13 14:58 EST (History)
7 users (show)

See Also:


Attachments
Profiler data (6.46 KB, application/zip)
2007-02-14 06:58 EST, Dani Megert CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dani Megert CLA 2007-02-14 06:57:21 EST
3.3 M5

I'm currently trying to open up file buffers for URIs but now run into a wall because getting the URI for an IPath (URIUtil.toURI(IPath)) is very expensive.
Comment 1 Dani Megert CLA 2007-02-14 06:58:43 EST
Created attachment 58960 [details]
Profiler data
Comment 2 Dani Megert CLA 2007-02-14 07:09:12 EST
NOTE: it is not just a compatibility issue: also clients that will use the new URI-based file buffer story will be much slower than now because they will call:

  bufferManager.getTextFileBuffer(myFile.getLocationURI());

which will be much more expensive (because of calling getLocationURI()) than

  bufferManager.getTextFileBuffer(myFile.getFullPath());

Hence both, the old story will be slower due to the migration layer in file buffers and the new API will also be slower because clients need to get the URI first.
Comment 3 Dani Megert CLA 2007-02-14 08:32:39 EST
This method is currently also heavily called by others in the system:
FileSystemResourceManager or by the SyncFileWriter to fetch CVS info.
Comment 4 John Arthorne CLA 2007-02-14 09:50:21 EST
Is the profile output in instrumentation or sampling mode?  What's the test case and VM?
Comment 5 John Arthorne CLA 2007-02-14 09:51:55 EST
*** Bug 174147 has been marked as a duplicate of this bug. ***
Comment 6 Dani Megert CLA 2007-02-14 10:03:44 EST
VM: 1.4.2_10
Used instrumentation (since the inv. count is there ;-) but same with sampling

Test Case:
path= full path of a valid resource in your test workspace
manager= FileBuffers.getTextFileBufferManager();
for (int i= 0; i < 100000; i++)
  manager.getTextFileBuffer(path);
Comment 7 Dani Megert CLA 2007-04-02 05:53:38 EDT
Can anything be done about this for 3.3? This method is called often and hence speeding it up would be a good improvement.

I'm setting the target milestone as this is the performance pass milestone. Please remove it if you think nothing can be done for 3.3.
Comment 8 John Arthorne CLA 2007-04-03 09:35:42 EDT
*** Bug 130736 has been marked as a duplicate of this bug. ***
Comment 9 John Arthorne CLA 2007-04-17 17:01:38 EDT
I don't see any obvious performance improvements in these methods. We could cache recently created URIs, but I'm not sure if that will have much effect (outside of the micro-benchmark test described in comment #6).  I did some instrumentation, and found that these methods are never called during typical resource manipulations (create, delete, copy, move, etc).  The method is called only once each time an editor is opened. Thus, I don't think any performance work in this method will have any impact on user scenarios.
Comment 10 John Arthorne CLA 2007-04-17 17:02:55 EDT
Please clarify if you think there is any noticeable improvement to be gained here.
Comment 11 Dani Megert CLA 2007-04-18 03:54:50 EDT
>I'm not sure if that will have much effect (outside of the micro-benchmark test >described in comment #6). 
Why do you say "micro" benchmark? Because it often runs through that method? e.g. file buffer operations (e.g convert line delimiter) that can run over every single file in the workspace are a real life scenario.

>The method is called
>only once each time an editor is opened. Thus, I don't think any performance
>work in this method will have any impact on user scenarios.
This is not true. Take your dev workspace make a full build and see how many times URIUtil.toURI(String) gets called, e.g. via Resource.getLocationURI() which was reported in bug 174147 that got (correctly) marked as duplicate of this one.

In addition see also bug 130736 which got reported by a different person and marked as dup of this one too.

>Please clarify if you think there is any noticeable improvement to be gained
>here.
I never said that - it just popped up prominent when investigating and hence thought there *might* be overall improvement and I also said in my mail that I might be wrong and if you now say this code is OK and has no room for improvement then feel free to close.
Comment 12 John Arthorne CLA 2007-04-18 09:57:33 EDT
Ok, I was just saying I didn't see anywhere it was being called often. Thanks for clarifying.
Comment 13 Eclipse Webmaster CLA 2019-09-06 16:08:53 EDT Comment hidden (obsolete)
Comment 14 Eclipse Genie CLA 2022-02-13 14:58:43 EST
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.