findleaves.py is too slow when doing a flame build

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kats, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

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?
Created attachment 8459685 [details]
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)
Created attachment 8459696 [details] [review]
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.
Attachment #8459696 - Flags: review?(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
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.
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 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).
Attachment #8459696 - Flags: review?(dhylands) → review+
(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.
Created attachment 8459801 [details]
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.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
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.
build/core/main.mk also uses findleaves.py, do we need to change it?
You need to log in before you can comment on or make changes to this bug.