joaoventura / pybridge

Reuse Python code in native Android applications
215 stars 56 forks source link

Eliminate resource leaks, and a string copy #3

Closed artwyman closed 7 years ago

artwyman commented 7 years ago

Came across these resource leaks while experimenting with Python on Android, and figured I should contribute. The leaks in init aren't a big deal since it runs once, but probably best to clean up properly when acting as en example. The original comment about the res buffer allocated with malloc was incorrect: NewStringUTF copies its input, without taking ownership. This version avoids an extra copy as well as that memory allocation.

Thanks so much for this sample. It greatly eased my attempts to try out Python on Android, once I discovered it after having trouble building Kivy. I've done some additional experimentation with error handling and performance timing in my fork, which you're welcome examine in my branch here. It's not cleaned up enough for publication, though, so I'm not going to try to push it.

joaoventura commented 7 years ago

Thanks for the PR, greatly appreaciated! 👍

So I've checked your "experiments" branch, I'll borrow some ideas from there if you don't mind. I think the call times are usually quite fast, but there as some low-hanging fruits regarding performance, e.g:

I would recommend not to leave many .py files in the assets/ folder as they will have to be copied to the device and that takes time (for instance, you moved some code of the log to a python file [android_log_redirect.py]). It is one more thing to import, which as I said above, can degrade performance on start if abused. Finally, I also suggest to use zipped python packages when possible (only one file to copy when the app starts first time and it usually imports faster).

artwyman commented 7 years ago

Thanks. Feel free to grab any of my experimental code you wish. I didn't try to make it pretty, but the error-handling and logging stuff was in the direction of what I'd likely want to write in future. If I go forward with this for product use I'll likely want to leverage C++ for error/resource handling and create something similar to the JNI support-library in Djinni here: https://github.com/dropbox/djinni/blob/master/support-lib/jni/djinni_support.hpp

In my basic performance testing my biggest worry was the startup time, at 690ms. Call time was .25-.5ms so less worrying, particularly given the overhead of the routing being included. I'm interested to hear that you think importing the logging code from a file would be significantly slower than creating the module from a string. Are you more worried about the asset unpacking time, or the actual Python import time?

Zipping up the python is a good tip. I'd also probably want to package pyo files instead of py files for maximal performance. As it is, this was a Hack Week project so I'm not going to push it forward much further for now, unless we decide to invest more heavily in future, which is still up in the air. I proved basic feasibility of Python on Android, though I wish I'd discovered pybridge sooner, rather than spending as much time as I did struggling with Kivy's tools, which aren't a great fit for my needs since I don't want their UI framework.

joaoventura commented 7 years ago

Are you more worried about the asset unpacking time, or the actual Python import time?

The asset unpacking time is, of course, a problem, but you can do something like the following to unpack only when the application is first installed or updated:

int APP_VERSION = BuildConfig.VERSION_CODE;
Boolean FORCE_EXTRACT = false;

if (!assetExtractor.existsAssets("python") 
        || assetExtractor.getAssetsVersion() < APP_VERSION
        || FORCE_EXTRACT) {             

    assetExtractor.removeAssets("python");
    assetExtractor.copyAssets("python");   
    assetExtractor.setAssetsVersion(APP_VERSION);
}

As for the import time, on my application I'm currently doing a splash screen, but it is because i'm importing pytz and a lot of other files. Eventually I would like to start the interpreter using an async task or something like that.. Other solutions are to import the necessary python packages as needed (for instance, inside the functions called by the router function). I don't think we can get much speed out of Py_Initialize()..

Zipping up the python is a good tip. I'd also probably want to package pyo files instead of py files for maximal performance.

I use pyc files inside the zip files and add them to the sys.path, here's an example of a script to build and zip it automatically. I can't remember now why I decided to use pyc instead of pyo files, but I don't think there's much practical difference (regarding performance and size, at least).

artwyman commented 7 years ago

I picked pyo because it's the "most optimized" and it's what Kivy uses (at least when it's not using CrystaX). From further reading, though, I think the only difference might be the removal of asserts, in which case I'd probably go with pyc anyway, since we generally keep asserts in our production code.