Introduce ExtensionVersioner for C++ extensions by goldsborough · Pull Request #11725 · pytorch/pytorch

goldsborough · 2018-09-14T23:59:45Z

Python never closes shared library it dlopens. This means that calling load or load_inline (i.e. building a JIT C++ extension) with the same C++ extension name twice in the same Python process will never re-load the library, even if the compiled source code and the underlying shared library have changed. The only way to circumvent this is to create a new library and load it under a new module name.

I fix this, of course, by introducing a layer of indirection. Loading a JIT C++ extension now goes through an ExtensionVersioner, which hashes the contents of the source files as well as build flags, and if this hash changed, bumps an internal version stored for each module name. A bump in the version will result in the ninja file being edited and a new shared library and effectively a new C++ extension to be compiled. For this the version name is appended as _v<version> to the extension name for all versions greater zero.

One caveat is that if you were to update your code many times and always re-load it in the same process, you may end up with quite a lot of shared library objects in your extension's folder under /tmp. I imagine this isn't too bad, since extensions are typically small and there isn't really a good way for us to garbage collect old libraries, since we don't know what still has handles to them.

Fixes #11398 CC @t-vi

@ezyang @gchanan @soumith @fmassa

t-vi · 2018-09-17T12:55:25Z

Thank you!

goldsborough · 2018-09-17T16:43:59Z

@peterjc123 could you do some digging for the Windows failure? I'm seeing

01:10:44 [2/2] "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\bin\Hostx86\x64/link.exe" main.o /nologo /DLL caffe2.lib torch.lib _C.lib /LIBPATH:C:\Jenkins\Miniconda3\libs /LIBPATH:C:\Jenkins\workspace\pytorch-builds\pytorch-win-ws2016-cuda9-cudnn7-py3-test2\test\torch\lib /out:reloaded_jit_extension_v1.pyd

01:10:44 FAILED: reloaded_jit_extension_v1.pyd 

01:10:44 "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\bin\Hostx86\x64/link.exe" main.o /nologo /DLL caffe2.lib torch.lib _C.lib /LIBPATH:C:\Jenkins\Miniconda3\libs /LIBPATH:C:\Jenkins\workspace\pytorch-builds\pytorch-win-ws2016-cuda9-cudnn7-py3-test2\test\torch\lib /out:reloaded_jit_extension_v1.pyd

01:10:44 LINK : fatal error LNK1104: cannot open file 'reloaded_jit_extension_v1.pyd'

01:10:44 ninja: build stopped: subcommand failed.

which means it can't link the second version for some reason. I don't quite understand why it only fails on Windows like this. It seems to compile the first version reloaded_jit_extension_v0.pyd fine but then fails for the second version.

peterjc123 · 2018-09-18T01:24:54Z

@goldsborough It's probably a permission issue. Is this library already in use during build?

goldsborough · 2018-09-18T01:49:55Z

@peterjc123 Sorry could you elaborate on what you mean?

peterjc123 · 2018-09-18T02:29:10Z

@goldsborough It seems that reloaded_jit_extension_v1.pyd is being used when compiling. On Windows, deleting a file that is used by another process is not allowed.

goldsborough · 2018-09-18T04:15:26Z

@peterjc123 I see. I don't know why that would be happening. It is going through the exact same code path as version 0, with the exact same function (_jit_compile). It's true that the version 0 library would be in use at the same time, and the ninja file is overwritten for version 1, but time-wise version 1 is built sequentially after version 0, so I don't think there would be interference there.

ezyang · 2018-09-18T04:29:48Z

One day, someone is going to deploy hot loadable C++ extensions with PyTorch in prod, and then wonder why after a week the Python process crashes XD

goldsborough · 2018-09-18T05:07:56Z

@ezyang as long as they don't modify the source code they reference from their JIT extension running in prod and re-load it, they should be ok :)

t-vi · 2018-09-18T05:33:59Z

They'd have a date stamp to be extra safe...

peterjc123 · 2018-09-18T06:47:04Z

@goldsborough Is reloaded_jit_extension_v1 built twice? When built for the first time, it should be okay from the log. But the second time, it would fail because it is still being used(loaded in python). We could not delete one file under this circumstances.
Possible solution:

Unload the DLL (impossible)
Append hash into filename (hard to load)
Generate libraries in new directories and use the latest one by appending PATHs or using some sorting method (feasible)
Skip this test (not good but it is the last way to go)

17:45:28 Building extension module reloaded_jit_extension...
17:45:28 Loading extension module reloaded_jit_extension...
17:45:28 Using C:\Windows\TEMP\torch_extensions as PyTorch extensions root...
17:45:28 Emitting ninja build file C:\Windows\TEMP\torch_extensions\reloaded_jit_extension\build.ninja...
17:45:28 Building extension module reloaded_jit_extension_v1... (first time: okay)
17:45:28 Loading extension module reloaded_jit_extension_v1...
17:45:28 Using C:\Windows\TEMP\torch_extensions as PyTorch extensions root...
17:45:28 Emitting ninja build file C:\Windows\TEMP\torch_extensions\reloaded_jit_extension\build.ninja...
17:45:28 Building extension module reloaded_jit_extension_v1... (second time: fail)
17:45:28 Traceback (most recent call last):
17:45:28   File "run_test.py", line 392, in <module>
17:45:28     main()
17:45:28   File "run_test.py", line 384, in main
17:45:28     raise RuntimeError(message)
17:45:28 RuntimeError: test_cpp_extensions failed!

goldsborough · 2018-09-19T20:50:12Z

This is looking healthy. Hooray! I think the one failing test is spurious. Let me retest it

facebook-github-bot

goldsborough has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

goldsborough · 2018-09-19T22:28:20Z

I made a change such that we don't even attempt to re-build an unchanged library. Previously we were just relying on Ninja for this, but now we know this messes with Windows due to access to a file from multiple processes. Thanks for your help @peterjc123.

facebook-github-bot

goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

goldsborough requested review from apaszke, colesbury, ezyang, fmassa, gchanan, soumith and zdevito as code owners September 14, 2018 23:59

soumith approved these changes Sep 15, 2018

View reviewed changes

goldsborough changed the title ~~Introduce ExtensionCache for C++ extensions~~ Introduce ExtensionVersioner for C++ extensions Sep 17, 2018

goldsborough force-pushed the reload-cpp-ext branch from 41162c3 to 9fff003 Compare September 17, 2018 16:38

goldsborough force-pushed the reload-cpp-ext branch 3 times, most recently from cc69f24 to 0ec2c32 Compare September 19, 2018 15:45

facebook-github-bot reviewed Sep 19, 2018

View reviewed changes

Introduce ExtensionCache for C++ extensions

9557341

goldsborough force-pushed the reload-cpp-ext branch from 0ec2c32 to 9557341 Compare September 20, 2018 16:20

facebook-github-bot reviewed Sep 20, 2018

View reviewed changes

facebook-github-bot closed this in 6100c0e Sep 20, 2018

ezyang added the merged label Jun 26, 2019

Conversation

goldsborough commented Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-vi commented Sep 17, 2018

Uh oh!

goldsborough commented Sep 17, 2018

Uh oh!

peterjc123 commented Sep 18, 2018

Uh oh!

goldsborough commented Sep 18, 2018

Uh oh!

peterjc123 commented Sep 18, 2018

Uh oh!

goldsborough commented Sep 18, 2018

Uh oh!

ezyang commented Sep 18, 2018

Uh oh!

goldsborough commented Sep 18, 2018

Uh oh!

t-vi commented Sep 18, 2018 via email

Uh oh!

peterjc123 commented Sep 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

goldsborough commented Sep 19, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

goldsborough commented Sep 19, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

goldsborough commented Sep 14, 2018 •

edited

Loading

peterjc123 commented Sep 18, 2018 •

edited

Loading