beeware / briefcase-android-gradle-template

A template for generating Android Gradle projects with Briefcase
MIT License
21 stars 22 forks source link

If pythonhome.zip has same filename, skip unpack #11

Closed paulproteus closed 4 years ago

paulproteus commented 4 years ago

This paves the way for the support library to use a filename that contains the hash of the stdlib's contents.

Testing performed

I launched a helloworld app twice. I watched adb logcat -s unpackPython. The first time, it printed:

05-28 23:20:20.445  6917  6917 D unpackPython: Unpacking Python with ABI x86 to /data/user/0/com.example.helloworld/files/python/stdlib

The second time, it printed:

05-29 00:28:47.865  7242  7242 D unpackPython: Skipping unpack of Python stdlb due to cache hit on pythonhome.x86.zip

It launched properly both times.

Testing remaining to be done

I want to validate this against a version of the Python support library which embeds the file hash into the filename, just to ensure it'll really work with that change.

Review status

Please review now if possible, and then I'll leave a comment once I'm done testing on my end.

paulproteus commented 4 years ago

Final testing done

I did the following testing.

05-29 00:50:11.770  7641  7641 D unpackPython: Unpacking Python stdlib due to cache miss on pythonhome.4d8852d041e83a4fc6b6daa7dc20ee2098f314786c7641318c722b7e5c001892.x86.zip
05-29 00:51:18.461  7682  7682 D unpackPython: Unpacking Python stdlib due to cache miss on pythonhome.4d8852d041e83a4fc6b6daa7dc20ee2098f314786c7641318c722b7e5c001892A.x86.zip
05-29 00:51:28.573  7721  7721 D unpackPython: Skipping unpack of Python stdlib due to cache hit on pythonhome.4d8852d041e83a4fc6b6daa7dc20ee2098f314786c7641318c722b7e5c001892A.x86.zip

I consider this a success for this PR. I may fine-tune the Python-Android-support change a tiny bit before submitting it for review.

paulproteus commented 4 years ago

Thanks for the review!

The other question that isn't addressed by your explanatory notes: Does this actually fix the problem? You've shown logs that demonstrate the cache is being hit - which is great - but how much effect does that have on launch time?

On my Linux install, it was approx 3 seconds. I/O from the Android emulator seems WAY faster here than on macOS, so I suspect it's more than 3 seconds on macOS.

I think I should change this so that we print timings to the debug log. I'll add that, then re-request review.

Do we need to explore doing the same thing to app packages and app code?

I don't know yet. Perhaps the best thing to do is for me to add timing debug log messages to those, so that we can see how long they are taking. I'll do that in a separate PR, probably.

I'm going to move this to "draft" status until I fix the above issues. Any other discussion is very welcome.