814767 - devicemanager: mkDirs optimization

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Geoff Brown [:gbrown]]

mkDirs() is a commonly used devicemanager function. Note particularly that it is called on every pushFile(). mkDirs() calls mkDir() for each component of the requested directory, and mkDir() first calls dirExists() to determine if that directory needs to be created.

For example, pushFile("/a/b/c/d/e"), when a/b/c/d exists, results in something like:

isdir a
isdir a/b
isdir a/b/c
isdir a/b/c/d
push a/b/c/d/e

We can easily reduce this case to:

isdir a/b/c/d
push a/b/c/d/e

If the directory does not exist, we can fall back to the current procedure.

Comment 1

[Geoff Brown [:gbrown]]

Attachment: in mkDirs, check for dir existence up front

Comment 2

[Geoff Brown [:gbrown]]

Try run is okay: https://tbpl.mozilla.org/?tree=Try&rev=35e21866f633

Comment 3

[Geoff Brown [:gbrown]]

Comment on attachment 684774 [details] [diff] [review]
in mkDirs, check for dir existence up front

This is not a big win, but it is simple and reduces dm <-> sut traffic a little.

Comment 4

[William Lachance (:wlach)]

Comment on attachment 684774 [details] [diff] [review]
in mkDirs, check for dir existence up front

This looks ok to me. I would prefer if you didn't use parantheses on the if statements, as that's the style we've decided to use elsewhere in this file. e.g.:

e.g. replace if (not self.dirExists(dirParts[0])):

with:

if not self.dirExists(dirParts[0]):

I know there's two places elsewhere in this function which use that style. Feel free to fix those as well.

Comment 5

[Geoff Brown [:gbrown]]

Attachment: in mkDirs, check for dir existence up front

Updated for review comments: removed 3 pairs of ()s. r=wlach

Comment 6

[William Lachance (:wlach)]

Pushed to mozbase: https://github.com/mozilla/mozbase/commit/5254c121d85ed17339985fec6f134c73f64d0583