This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 194683

Summary: Editing checked out files unusable slow
Product: platform Reporter: Erno Mononen <emononen>
Component: FilesystemsAssignee: Tomas Stupka <tstupka>
Status: RESOLVED FIXED    
Severity: normal CC: apireviews, mmetelka, tstupka
Priority: P3 Keywords: API_REVIEW_FAST
Version: 7.0   
Hardware: PC   
OS: Other   
Issue Type: DEFECT Exception Reporter:
Attachments: profiler snapshot
possible fix for FS
suggested patch - masterfs
suggested patch - masterfs
suggested patch - VCS
masterFS
VCS raised module version
masterfs (fixed Y01..03)

Description Erno Mononen 2011-01-25 11:53:26 UTC
Created attachment 105329 [details]
profiler snapshot

NetBeans IDE Dev (Build 201101160000), WinXP, JDK 1.6.0_23

With a ClearCase dynamic view and a checked out file, editing or just moving the caret around in the file is unusably slow. This seems to boil down to the fact that File#canWrite is consistently slow for such files and the IDE invokes it multiple times in the EDT. 

Steps to reproduce:
1. Open a project from a dynamic view
2. Check out a file
3. Move the caret around in the file => observe the slowness

Attached is a snapshot.
Comment 1 Tomas Stupka 2011-01-25 17:12:34 UTC
tow main problems in this issue:

1.) unnecessary file.canWrite() calls in filesystem
in case there is more than 1 ProvidedExtensions instance then o.n.m.masterfs.ProvidedExtensionsProxy.canWrite(File) should stop iterating after the first canWrite() which returns true. This way we will:
a.) reduce the amount of file.canWrite() calls
b.) prevent the not intended misbehavior where canWrite is evaluated as false even if some ProvidedExtensions returned true 

2.) too many fileObject.canWrite() calls from editor
Comment 2 Tomas Stupka 2011-01-25 17:29:39 UTC
Created attachment 105337 [details]
possible fix for FS
Comment 3 Tomas Stupka 2011-01-25 17:38:49 UTC
a better solution than suggested in patch would be to always bypass Watcher.canWrite() in o.n.m.masterfs.ProvidedExtensionsProxy.canWrite(File)
Comment 4 Jaroslav Tulach 2011-01-25 22:27:33 UTC
Basically you need three states: yes/no/don'tknow. Right? Change the canWrite method to return Boolean then! Or rather (to remain compatible) create 

protected Boolean isWriteable() { return canWrite(); } 

and make sure ProvidedExtensionsProxy calls the isWriteable method. In Watcher return null. In ProvidedExtensionsProxy iterate until you find non-null value.
Comment 5 Tomas Stupka 2011-01-26 16:36:36 UTC
> protected Boolean isWriteable() { return canWrite(); } 
> and make sure ProvidedExtensionsProxy calls the isWriteable method.
protected wouldn't be accesible from ProvidedExtensionsProxy so there also had to be a kind of accessor type (or method) ...

personally i would prefer the incompatible change:
- the api is friend and masterfs and VCS are the only implementors of ProvidedExtension 
- using boolean instead of Boolean was a design mistake - lets fix it
 
> In ProvidedExtensionsProxy iterate until you find non-null value
i would iterate until somebody returns true as that was the reason for the canWrite() method in the beginning -> to eventually override a false from somebody else - e.g. the filesystem
Comment 6 Miloslav Metelka 2011-01-27 13:16:58 UTC
Editor part: fix that reduces querying of file.canWrite() from ReadOnlyFilesHighlighting:
http://hg.netbeans.org/jet-main/rev/924ff3358f2a
Comment 7 Jaroslav Tulach 2011-01-27 15:48:17 UTC
(In reply to comment #5)
> personally i would prefer the incompatible change:
> - the api is friend and masterfs and VCS are the only implementors of
> ProvidedExtension 
> - using boolean instead of Boolean was a design mistake - lets fix it

Then it is necessary to change the major version of the module to signal to 6.9.x modules that they cannot link with new versions anymore.

> > In ProvidedExtensionsProxy iterate until you find non-null value
> i would iterate until somebody returns true as that was the reason for the
> canWrite() method in the beginning -> to eventually override a false from
> somebody else - e.g. the filesystem

I am slightly confused after reading the previous paragraph: why we need three state Boolean if you only seek for Boolean.TRUE? If you see null or Boolean.FALSE you'd continue to next ProvidedExtension. What is the need for three states then?
Comment 8 Quality Engineering 2011-01-28 06:05:31 UTC
Integrated into 'main-golden', will be available in build *201101280000* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/924ff3358f2a
User: Miloslav Metelka <mmetelka@netbeans.org>
Log: #194683 - Editing checked out files unusable slow - Editor part: fix that reduces querying of file.canWrite() from ReadOnlyFilesHighlighting.
Comment 9 Erno Mononen 2011-02-01 12:02:50 UTC
I tested with the latest nightly and after Mila's fix things are much better now - thanks for the quick fix! So from my point of view this is no longer a P1 issue.
Comment 10 Tomas Stupka 2011-02-01 13:30:45 UTC
(In reply to comment #9)
> I tested with the latest nightly and after Mila's fix things are much better
> now - thanks for the quick fix! So from my point of view this is no longer a P1
> issue.
thanks
Comment 11 Tomas Stupka 2011-02-01 15:11:20 UTC
after a discussion with jarda we decided to solve to problem by calling canWrite only for ProvidedExtensions instances which are meant to provide such value (e.g. VCS) and to ignore instances which aren't (e.g. Watcher)
Comment 12 Tomas Stupka 2011-02-01 15:14:57 UTC
Created attachment 105551 [details]
suggested patch - masterfs
Comment 13 Tomas Stupka 2011-02-01 15:15:02 UTC
Created attachment 105552 [details]
suggested patch - masterfs
Comment 14 Tomas Stupka 2011-02-01 15:16:14 UTC
Created attachment 105553 [details]
suggested patch - VCS
Comment 15 Tomas Stupka 2011-02-01 15:39:49 UTC
Created attachment 105554 [details]
masterFS

raised module version
Comment 16 Tomas Stupka 2011-02-01 15:40:25 UTC
Created attachment 105555 [details]
VCS 
raised module version
Comment 17 Jaroslav Tulach 2011-02-01 21:05:52 UTC
Y01 Summary of the apichange is sort of wrong.
Y02 I don't understand the description either: what shall I "setup"?
Y03 I can see a change in tests, but it does not look like a real test.
Comment 18 Tomas Stupka 2011-02-02 13:47:32 UTC
(In reply to comment
Comment 19 Tomas Stupka 2011-02-02 13:51:06 UTC
> Y01 Summary of the apichange is sort of wrong.
indeed. wil fix

> Y02 I don't understand the description either: what shall I "setup"?
will fix wording

> Y03 I can see a change in tests, but it does not look like a real test.
right. the test was previously changed only not to break the already existing test testImplsCanWrite. will enhance it so that both scenarios are tested - when providesCanWrite is true and also when it's false.
Comment 20 Tomas Stupka 2011-02-02 14:01:37 UTC
Created attachment 105596 [details]
masterfs (fixed Y01..03)
Comment 21 Tomas Stupka 2011-02-04 01:01:35 UTC
fixed in http://hg.netbeans.org/core-main/rev/576567bc14b1
Comment 22 Quality Engineering 2011-02-05 06:10:18 UTC
Integrated into 'main-golden', will be available in build *201102050000* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/576567bc14b1
User: Tomas Stupka <tstupka@netbeans.org>
Log: issue #194683 - Editing checked out files unusable slow