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

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

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.

Comment 1

[Brian R. Bondy [:bbondy]]

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.

Comment 2

[[e-mail]]

(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.

Comment 3

[Brian R. Bondy [:bbondy]]

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>\*

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2.

Comment 5

[Brian R. Bondy [:bbondy]]

Note: You can use FindFirstFileW on an actual file but not when you append \\* as we always do.

Comment 6

[[e-mail]]

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!

Comment 7

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2df4fa51cfbb

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2df4fa51cfbb