1041635 - findleaves.py is too slow when doing a flame build

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

When doing a ./build.sh gecko for a flame device, a large amount of time (upwards of a minute) is spent running the findleaves.py script to find some files in the tree. This seems rather unreasonable considering that the gnu utility "find" can do the same job in a few seconds. Is there any particular reason this is handwritten in python rather than just using the find utility?

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: find vs findleaves.py

find:
real	0m1.340s
user	0m0.520s
sys	0m0.644s

findleaves.py:
real	3m20.350s
user	0m36.080s
sys	2m43.812s

Also, findleaves.py missed a file (the one in frameworks/base/packages/WAPPushManager/CleanSpec.mk)

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Github PR to b2g-4.3_r2.1 branch

This fixes the slow ./build.sh gecko for me. I didn't bother changing the other findleaves.py calls because they don't get invoked as part of ./build.sh gecko and they look more complicated to change.

Comment 3

[Dave Hylands [:dhylands]]

I commented on github, but I figured I'd comment here as well.

Shouldn't the find command also exclude .repo and out directories?

Something like this:

> find . ! \( \( -name .repo -o -name out \) -prune \) -name CleanSpec.mk

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Yeah I guess so, to be safe. None of those directories actually contain any CleanSpec.mk files on my machine but I updated the PR to exclude them. I also excluded .git folders to match what the original code was doing.

Comment 5

[Dave Hylands [:dhylands]]

My builds don't have anything in out, but each file in the rest of the tree had a corresponding entry in .repo:

> ./device/common/CleanSpec.mk
> ./device/sample/CleanSpec.mk
> ./.repo/development/tools/emulator/opengl/tests/gles_android_wrapper/CleanSpec.mk
> ./.repo/development/CleanSpec.mk
> ./.repo/device/common/CleanSpec.mk
> ./.repo/device/sample/CleanSpec.mk
> ./.repo/device/samsung/crespo/CleanSpec.mk
> ./.repo/device/samsung/galaxys2/CleanSpec.mk
...remainder snipped...

Comment 6

[Dave Hylands [:dhylands]]

Comment on attachment 8459696 [details] [review]
Github PR to b2g-4.3_r2.1 branch

This looks good to me.

You should test on the Mac to make sure that the Mac find command behaves the same as the linux one.

r=me with that test done (by somebody).

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(In reply to Dave Hylands [:dhylands] from comment #5)
> My builds don't have anything in out, but each file in the rest of the tree
> had a corresponding entry in .repo:

That's not the case in any of my 6 B2G repos (across two machines). Perhaps you misconfigured yours at some point and it checked stuff out where it shouldn't have? Regardless I suppose this might happen to other people so it's good to check for it.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: find vs findleaves.py on Mac OS

On my Hamachi tree checked out on OS X, "find" found two extra CleanSpec.mk files over the findleaves version:

development/tools/emulator/opengl/tests/gles_android_wrapper/CleanSpec.mk
frameworks/base/packages/WAPPushManager/CleanSpec.mk

Other than that the same files were found, and the command seems to work fine.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #8)
> On my Hamachi tree checked out on OS X, "find" found two extra CleanSpec.mk
> files over the findleaves version:
> 
> development/tools/emulator/opengl/tests/gles_android_wrapper/CleanSpec.mk
> frameworks/base/packages/WAPPushManager/CleanSpec.mk

Ah, it looks like this behaviour is intentional. findleaves.py aborts the search in a subtree if it finds a CleanSpec.mk. So for the WAPPushManager example above, there is a CleanSpec.mk at frameworks/base/CleanSpec.mk, so findleaves.py doesn't go into the packages/ subdir. From the comment at the top of findleaves.py this sounds like it is intentional although I don't know enough about the build system to say what purpose this serves.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

The WAPPushManager cleanspec.mk file is empty anyway, and the gles_android_wrapper one refers to a file that doesn't exist in my out/ directory. In general I think running extra clean files should be safe enough; in the worst case it'll clean too much and will require more stuff to be built but it shouldn't break anything.

Dave, could you merge the PR if you're happy with this? I don't have permissions on the github repo.

Comment 11

[Dave Hylands [:dhylands]]

b2g-4.3_r2.1:
https://github.com/mozilla-b2g/platform_build/commit/3aa6abd313f965a84aa86c6b213dc154e4875139

b2g-4.4.2_r1:
https://github.com/mozilla-b2g/platform_build/commit/0d616942c300d9fb142483210f1dda9096c9a9fc

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

build/core/main.mk also uses findleaves.py, do we need to change it?