Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support wheel building and speed up IOTile tool #645

Merged
merged 10 commits into from
Jan 21, 2019
Merged

Conversation

timburke
Copy link
Member

There are four main effects of this PR:

  1. The process by which support wheels are built in iotile-build is extended to support more complicated packages that contain many files. Previously, it was assumed that there would only be a few python modules in a support wheel that were just directly copied and bundled together as part of the build process. However, this did not allow for those modules to import other modules reliably since the load_extensions() method of importing the modules during development would not create a proper package structure to allow for relative imports.

    As support packages get more complicated because they include tile emulators, it's become important to allow for multiple modules that import each other so the python code in the support packages can be more easily maintained. To this end, an IOTile component can now declare a support_package product that is a normal python package. This package will become the support wheel and when individual entry points inside of it are loaded during development they will be loaded as proper submodules so that they can import each other as you would expect.

    This makes complex support packages inside of a component much easier to design and test since they are just standard python packages. Importantly, they are always imported using their fully unique support distribution name, even during development so that there are not collisions if multiple development components all put their support packages in the python/support directories (causing module shadowing on the support global package name).

  2. It incrementally enhances iotile-emulate to allow for returning hardware versions and improves SerializableState to allow for serializing more complicated state trees without needing to specify explicit serializers and deserializers. There is now support for serializing lists and dicts of objects automatically.

  3. It removes all explicit references to pkg_resources from iotile-core and iotile-build, moving them both to using faster alternatives (the entrypoints package for entry points). This is done primarily to allow for faster startup times in the iotile tool, which is quite slow on machines with slow disks such as those with SD cards. See Consider migrating to importlib_resources for finding data file in packages #644 and Add ability to freeze list of iotile plugins #472. As part of this process the IOTile tool loading process was profiled and key culprits that are particularly slow to import (~ 100 ms each) are deferred. This was past.builtins and websockets. Neither was required to be loaded eagerly. In testing, these changes were able to speed up the iotile tool's startup time by more than 2x.

  4. It adds experimental support for speeding up the iotile tool in embedded contexts where the list of installed plugins is static. In that scenario, there is no need to enumerate all installed packages looking for plugins since the list will not change. You can instead just cache a list of plugins and look at that list instead. Two new routines iotile registry freeze and iotile registry unfreeze are added to cache and remove the cache of plugins, respectively. These methods are not yet ready for primetime use. In particular, the rest of CoreTools needs to move completely off of pkg_resources before the cache will work completely.

Copy link
Contributor

@mattrunchey mattrunchey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

We might eventually want to standardize the way we use sys.version_info. Doesn't seem to be a dominant pattern in coretools, https://github.com/iotile/coretools/search?q=sys.version_info&unscoped_q=sys.version_info

@timburke
Copy link
Member Author

We might eventually want to standardize the way we use sys.version_info. Doesn't seem to be a dominant pattern in coretools, https://github.com/iotile/coretools/search?q=sys.version_info&unscoped_q=sys.version_info

@mattrunchey Agreed. My general philosophy has been to avoid it whenever possible and rely on the future package to cleanly handle any required backports. In general that has worked quite well, with the only exception being what I discovered with past.builtins over the weekend.

I did import profiling using import_profiler and found that just importing past.builtins caused 100-200 ms delay in the startup time of the iotile tool because apparently that module is implemented by calling 2-to-3 at import time to analyze some code and dynamically generate the module in some way.

Ideally, we would just blacklist past.builtins and find a way to never need it but I have not found a cleaner alternative yet. The only use-case we have for it is when we need to check if something is a string in order to trigger string conversion to some type when we could also receive the type directly. In that case we need something like a isinstance(x, str) but that breaks on python 2 since depending on where the string came from it could be str or unicode. However, unicode doesn't exist on python 3. So the workaround we settled on was isinstance(x, basestring) with the past.builtins port for python 3. However, now it looks like that is super slow to import so we may need to write a utility module to check for strings that has a python 2 / python 3 branch inside and replace all basestring usage with that.

I'm making an issue for this.

@timburke
Copy link
Member Author

Opened #647 to track the past.builtins issue.

@timburke timburke merged commit 727d1b5 into master Jan 21, 2019
@timburke timburke deleted the fix-coretools branch January 21, 2019 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants