Closed Bug 726481 Opened 10 years ago Closed 9 years ago

Reduce calls to nsLocalFileWin's IsDirectory by not calling it when enumerating directories

Categories

(Core :: XPCOM, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: bbondy, Assigned: bbondy)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v1. (obsolete) — Splinter Review
IsDirectory is called when enumerating directories unnecessarily.  
We can instead check it after the call to FindFirstFile which automatically returns the file attributes with it.
Attachment #596530 - Flags: review?(neil)
Most of the time when this function is called it is called on a directory, so in that case we basically do 2 calls to fetch file attributes.
(In reply to Brian R. Bondy from comment #0)
> IsDirectory is called when enumerating directories unnecessarily.  
> We can instead check it after the call to FindFirstFile which automatically
> returns the file attributes with it.
But that returns the attributes of the found file, not the directory that we're enumerating. If people subsequently call IsDirectory on the found file, then yes, there is potential for optimisation there, but it would be nontrivial.
The first found file of a directory should be "." or the directory itself so this patch should work. 

Actually I think we can simplify this patch even more because OpenDir always adds \\* to the path so it would always return an error for a file. 

FindFirstFile returns ERROR_DIRECTORY if you pass <filepath>\* and it returns ERROR_PATH_NOT_FOUND when you have a <filepath_that_doesn't_exist>\*
Attached patch Patch v2.Splinter Review
Attachment #596530 - Attachment is obsolete: true
Attachment #596530 - Flags: review?(neil)
Attachment #596663 - Flags: review?(neil)
Note: You can use FindFirstFileW on an actual file but not when you append \\* as we always do.
Comment on attachment 596663 [details] [diff] [review]
Patch v2.

Bah, I'm disappointed that bug 359808 didn't remove all the nsDir stuff from bug 162361 particularly given that they had the same author and reviewer!
Attachment #596663 - Flags: review?(neil) → review+
https://hg.mozilla.org/mozilla-central/rev/2df4fa51cfbb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.