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

Windows wheel builds run out of memory #10074

Closed
JukkaL opened this issue Feb 11, 2021 · 21 comments
Closed

Windows wheel builds run out of memory #10074

JukkaL opened this issue Feb 11, 2021 · 21 comments
Labels
bug mypy got something wrong priority-0-high

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 11, 2021

Windows wheel builds started running out of memory. Here's an extract from the logs:

C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.28.29333\bin\HostX86\x64\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -IC:\Users\RUNNER~1\AppData\Local\Temp\pip-req-build-b3o88uhy\mypyc\lib-rt -Ibuild -Ic:\cibw\python\python.3.5.4\tools\include -Ic:\cibw\python\python.3.5.4\tools\include "-IC:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.28.29333\ATLMFC\include" "-IC:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.28.29333\include" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\shared" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\um" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\winrt" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\cppwinrt" /Tcbuild\__native_35888ab3ae83eb1dd0bf.c /Fobuild\temp.win-amd64-3.5\Release\build\__native_35888ab3ae83eb1dd0bf.obj /O2 /wd4102 /wd4101 /wd4146 /GL- /wd9025
  cl : Command line warning D9025 : overriding '/GL' with '/GL-'
  cl : Command line warning D9014 : invalid value '9025' for '/wd'; assuming '5999'
  __native_35888ab3ae83eb1dd0bf.c
  build\__native_35888ab3ae83eb1dd0bf.c(4558): fatal error C1060: compiler is out of heap space
  error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.28.29333\\bin\\HostX86\\x64\\cl.exe' failed with exit status 2
  Building wheel for mypy (PEP 517): finished with status 'error'
  ERROR: Failed building wheel for mypy
@JukkaL JukkaL added bug mypy got something wrong priority-0-high labels Feb 11, 2021
@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 11, 2021

It looks like this started happening after the 0.800 release, so this doesn't block the 0.810 release.

To investigate this further, we can try bisecting the commits since 0.800 was released. This might have been caused by the organic growth of the compiled codebase, or by a mypyc regression. If my first hypothesis is correct, a possible workaround is to not compile some parts of the codebase to get the build again under the CI memory limit.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 11, 2021

This started happening some time after 68a0c65. After this commit wheels builds were broken, so we don't know which commit caused the failure. There is at least one suspicious mypyc commit which is worth checking (7ec1455).

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Feb 11, 2021

There was some Windows code that I removed when writing the new wheel build script, because I didn't understand it and it worked without it. In case it helps: https://github.com/mypyc/mypy_mypyc-wheels/blob/5638de415c9fc7fa9076b2d9a429e887817943a0/appveyor.yml#L32-L39 (mentioned in #9536 (comment))

@ethanhs
Copy link
Collaborator

ethanhs commented Feb 11, 2021

Ah the important part is:
- call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvars64.bat"

By default MSVC uses a 32 bit compiler executable, which often OOMs on larger compiles, vcvars64.bat sets up the environment to use the 64 bit compiler executable.

E: Also is it correct to remove "SET MYPYC_OPT_LEVEL=2"?

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Feb 11, 2021

The default is MYPYC_OPT_LEVEL=3, which seemed better / consistent with what we do on other platforms (https://github.com/mypyc/mypy_mypyc-wheels/blob/a6cee7828a842ac2ffd2c5fafc7f2227094b0923/.github/workflows/build.yml#L60). Since things just worked when I set it up, I didn't mess around further.

@python python deleted a comment from missa112 Feb 15, 2021
@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 15, 2021

I vaguely remember that using MYPYC_OPT_LEVEL=3 and a 32-bit compiler caused problems on Windows (slow compiles and/or running out of memory). It's possible that we are now using a more recent compiler and things got better, but only barely so -- and now we have hit some code size threshold and regressed again.

Switching to consistent settings was a good idea, since it did work. However, I think that we should now try reverting to the original settings to get the build working again. I can start by trying to switch the optimization level on Windows.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 15, 2021

Changing the optimization level didn't help. It would be great if somebody can figure out how to invoke vcvars64.bat in the GitHub action.

@ethanhs
Copy link
Collaborator

ethanhs commented Feb 15, 2021

I believe it should simply be

    - shell: cmd
      run: |
          call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
          python setup.py build

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 15, 2021

Since we build all platforms using the same file, I think that we need to skip this on non-Windows platforms. Maybe we can use an if statement?

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 15, 2021

I can't figure how to use an if statement for this -- it might not work.

Another idea would be to use pwsh as the shell, since it should be supported on all platforms. I've never used PowerShell, however. I guess we can also always use a Python script.

@ethanhs
Copy link
Collaborator

ethanhs commented Feb 15, 2021

You can add an if: startsWith(matrix.os, 'windows') to the item.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Feb 15, 2021

Yup, adding

- shell: cmd
  run: call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
  if: startsWith(matrix.os, 'windows-')

as a step around here seems like it should work: https://github.com/mypyc/mypy_mypyc-wheels/blob/818f2887a15eb0f3d660f959123977421a6ed5f5/.github/workflows/build.yml#L35

@ethanhs
Copy link
Collaborator

ethanhs commented Feb 16, 2021

Note I believe you need to have both the call to vcvarsall 64.bat and the setup.py build in the same block as each command gets run in a fresh process.

JukkaL added a commit to mypyc/mypy_mypyc-wheels that referenced this issue Feb 16, 2021
This removes inline configuration and moves it to a script.
This makes it easier to have more complex logic during the build,
and also we can more easily test the build locally.

Also there's less need to understand the details of how GitHub actions
are configured.

Related to python/mypy#10074.
@JukkaL JukkaL closed this as completed in 4c3e250 Feb 16, 2021
@JukkaL JukkaL reopened this Feb 16, 2021
@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 16, 2021

Closed this accidentally. I moved the wheel build logic to a script in the mypy repository (#10096). This should make it easier to experiment with things and to add more complex logic. Once the new script works as well as the old build logic, I'll try to finally fix the Windows wheel build.

JukkaL added a commit to mypyc/mypy_mypyc-wheels that referenced this issue Feb 16, 2021
This removes inline configuration and moves it to a script.
This makes it easier to have more complex logic during the build,
and also we can more easily test the build locally.

Also there's less need to understand the details of how GitHub actions
are configured.

Related to python/mypy#10074.
JukkaL added a commit that referenced this issue Feb 16, 2021
This will hopefully fix out of memory errors.

Work on #10074.
JukkaL added a commit that referenced this issue Feb 16, 2021
This will hopefully fix out of memory errors.

Work on #10074.
@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 16, 2021

Running vcvars64.bat before invoking cibuildwheel didn't help. The 32-bit compiler seems to be still used.

I'm running out of options. I would appreciate any help here, particularly since I don't have a Windows dev environment set up, so debugging issues is slow. With the most recent changes, it should be possible to test things locally. I tried some things in mypyc/mypy_mypyc-wheels#20, but they didn't help.

Here are a few other ideas that might plausibly help:

  1. Use VsDevCmd.bat instead of vcvars64.bat.
  2. Maybe environment variables don't get propagated somehow from vcvars64.bat. Make sure this works as expected. If not, figure out what needs to be done to propagate them. If nothing else helps, updating CIBW_ENVIRONMENT_WINDOWS might do the trick.
  3. Create a separate workflow for Windows wheels that doesn't use cibuildwheel, similar to what we had before in Appveyor.
  4. Maybe we just don't have enough memory in the worker, and using the 32-bit compiler is a red herring?

@Akuli
Copy link
Contributor

Akuli commented Feb 16, 2021

In the past, I have used this github action.

@hauntsaninja
Copy link
Collaborator

I was surprised mypyc/mypy_mypyc-wheels#20 didn't work. Try setting DISTUTILS_USE_SDK=1? (Agreed that this is easiest done by someone with access to a Windows dev machine)

JukkaL added a commit to mypyc/mypy_mypyc-wheels that referenced this issue Feb 18, 2021
We have been running out of memory during Windows wheel builds and this
will hopefully fix it.

See python/mypy#10074 for more context.
@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 18, 2021

Try setting DISTUTILS_USE_SDK=1?

This seems to enable the 64-bit compiler, but we still run out of memory.

My next attempt is reducing the size of the build: #10107

If that doesn't help, bisecting to find the commit that caused the breakage could be helpful. This will be somewhat tricky, since we'll need to merge changes to the build scripts on top of the mypy commit to test before the build works.

I'm also working on reducing the size of the generated C code, which may also help with memory use. This will take some time, however, but this could be a good longer-term solution.

If nothing else works, we can perhaps split the build into multiple smaller files, or get a bigger machine for running Windows builds.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 19, 2021

Reducing the size of the build did not help. The build was already using multiple files.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 19, 2021

It now seems like 7ec1455 (or the follow-up commit) caused the failures. I've reverted the commit now.

My initial impression is that MSVC doesn't like the code we are generating and runs out of memory, either because of some major inefficiency or a bug. I have an idea about how to modify the generated code to not cause problems. I will have to verify that the Windows wheel build no longer is affected before landing the change again.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 19, 2021

We now have Windows wheels again: https://github.com/mypyc/mypy_mypyc-wheels/releases/tag/v0.820%2Bdev.b07c564043328ded5686d1aac4d7d3e5c8a5bfb0

@JukkaL JukkaL closed this as completed Feb 19, 2021
JukkaL added a commit that referenced this issue Feb 19, 2021
The contents are extracted from the current GitHub action definition:
https://github.com/mypyc/mypy_mypyc-wheels/blob/master/.github/workflows/build.yml

This is a fairly direct translation of the existing behavior. The
behavior should be identical to the current workflow.

The idea is to make this easier to maintain and easier to test
locally.

This should also make it easier to fix #10074.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong priority-0-high
Projects
None yet
Development

No branches or pull requests

4 participants