Fixing long PYTHONPATH and the resulting performance issues

[instinct-vfx]

Hi all,

when using many python packages in a resolve there are two very common issues (on windows at least):

• long PYTHONPATH that is eventually truncated if it gets too long due to both environment variable limits as well as shell limits.
• Performance issues. Specifically with many packages on PYTHONPATH starting python in the resolved shell can take quite a while.

For reference, with a set of 100 test packages that do nothing except for being importable starting python takes 18s (with a local python to reduce the impact of python itself).

The proposed fix reduces this time to 1.8s.

Credits: This approach was originally created by our VFX department. Kudos.

I would like other people to test this and discuss any potential disadvantages or problems this approach might cause.

The basic approach is as follows:

• Create a temporary folder
• Create a USER_SITE compatible folder structure inside
• Add a .pth file with all paths on PYTHONPATH
• Unset PYTHONPATH
• Set PYTHONUSERBASE to temporary folder

This can be applied using different ways. Specifically it could be part of the python package itself, or for testing as we did for now as a separate package doing the above in post_commands. Here is a sample implementation of said post_commands function:

def post_commands():
    import os
    import tempfile
    # get a compatible site.PYTHONUSERBASE python folder based on current platform
    # https://docs.python.org/3/library/site.html#site.USER_SITE
    usersite_python = {'major': env.REZ_PYTHON_MAJOR_VERSION,
                       'minor': env.REZ_PYTHON_MINOR_VERSION,
                       'separator': '.' if system.platform in ['linux', 'osx'] else '',
                       'osx': '/' if system.platform == 'osx' else ''
                       }
    python_version = "python{osx}{major}{separator}{minor}".format(
        **usersite_python)
    resolve_dir = tempfile.mkdtemp(prefix='rez_python_env')
    # create a site.USER_SITE compatible directory
    pth_dir = os.path.join(resolve_dir, python_version, 'site-packages')
    if not os.path.exists(pth_dir):
        os.makedirs(pth_dir)
    # add every module from PYTHONPATH inside a .PTH file
    pth_file = os.path.join(pth_dir, 'packages.pth')
    path_list = str(env.PYTHONPATH).split(os.pathsep)
    with open(pth_file, 'w') as f:
        for path in path_list:
            path = path.replace("\\", "/").replace("/", os.path.sep)
            f.write('{}\n'.format(path))
    # clean PYTHONPATH and set site.PYTHONUSERBASE variable
    env.PYTHONPATH.unset()
    env.PYTHONUSERBASE.set(resolve_dir)

Any thoughts, tests, feedback would be greatly appreciated. This can also serve as a test for the new GitHub discussions :)

[nerdvegas]

I'm keen to try this out!

One thing: It might be useful to expose the already existing temporary context dir in package commands. That way you could create your temp dir within that, so it gets cleaned up when the context exits.

[zachlewis]

Very clever! I imagine this could really improve user experience with certain python-based command-line utilities...

I often set PYTHONUSERBASE in pre_build_commands to the build install path (e.g., #892)... I imagine overriding PYTHONUSERBASE in the post_commands of the python package might contraindicate with such use? Maybe PYTHONUSERBASE can be set in post_commands only if not building.

[mstreatfield]

I seem to remember that we did something similar at Animal Logic - @fnaum might be able to comment on whether it's still used and how well it does (or doesn't) work.

Our trigger for doing this was hitting a limit in Maya; once the PYTHONPATH reached 4096 characters long a .so in Maya began to reject it (I want to say it was something to do with flexlm) and Maya refused to start. We didn't follow your approach exactly (at the time anyway) as post_commands didn't exist and we weren't using .pth files, however it is essentially the same idea (and resulted in similarly impressive performance improvements).

When do the temporary directories get cleaned up, I think mkdtemp requires the user to manually cleanup at some point?

Would you also look to apply this to other packages that are managed through a *PATH variable? I think we tried it with .so files and the LD_LIBRARY_PATH although the performance improvement was less significant; again, @fnaum may be able to comment more here.

[fnaum]

Hi,

Hey @mstreatfield you are correct, AL implementation is different, it creates a temp folder, and then it goes thru all the directories in order of the *PATH variable and it creates a symlink to each item on it, if the item already exists it is omitted, the variable is pointed at this temp directory, that way we avoid thousands of missing file stats.

Regarding, PYTHONPATH we still have not enabled this by default, many people use that locally and the performance is quite significant. But last time we visited this, this method had problems on the farm. I will get the details and share them.

On the other hand, we have been using the flattening of LD_LIBRARY_PATH always on for several years now (~5). I will still say that the performance improvements are still quite significant.
At some point due to some USD particular packages, we implemented recursive flattening (going recursively thru folders that appear on the LD_LIBRARY_PATH ). That made those USD workflows to work but brought an issue. The temp folder created (and not cleaned) did not use any storage space but it consumed thousands of inodes per rez env, mostly due to flattening the python package lib folder. I'll double-check but I think we ended up doing something not elegant like omitting recursive flattening for python. But, apart from that issue, we have not run into any issue with LD_LIBRARY_PATH.

@nerdvegas I think we have a good reason behind creating and not cleaning it up right away. I remember we did something explicit to prevent the cleanup of those directories (on maya/houdini/nuke environments) and left the cleanup up to system level to delete them when they are X days old. I'll try to dig on the old docs to see If I can share the exact reason why that was needed.

Fede

[fnaum]

Found this piece of code in the Maya and Nuke launchers

# Force rez to not delete the temporary folders/files in /tmp
# as they are still being used by the subshell which is created.
try:
    resolvedContext.disable_tmpdir_cleanup()
except Exception as e:
    pass

[Jawabiscuit]

I ran across something interesting in respect to PATH and executables that might be worth considering IDK but chocolatey has a ShimGen tool, https://docs.chocolatey.org/en-us/features/shim I wonder if there's some ready-made cross-platform way of shimming binaries. There seem to be plenty of benefits, see the link for more info. It would probably be especially worth looking into for windows.

[herronelou]

Were there any plans to build this into rez sometime in the future?

[ColinKennedy]

See bottom of #1242

"So then it would be common for python packages to define a "python" anchor at {root}/python. Thus the various package commands like env.PYTHONPATH.append('{root}/python') would collapse into a single path appended to PYTHONPATH."

The suggestion in the modularity discussion differs from what is suggested here but it addresses the same problem.

[instinct-vfx]

This (as in my original post above) is not really a feature of rez, but a feature of the python package. There might be a best practice python package. Or 10 if everything goes according to xkcd ;)

As for the modularity discussion this might move parts of this into a runtime engine. That would likely still bot be rez core, but it would also not be python specific anymore.