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

High CPU usage from mono-sgen building windows without asar on OS X #351

Closed
ArekSredzki opened this issue Apr 21, 2016 · 24 comments
Closed
Labels

Comments

@ArekSredzki
Copy link

Hi
Today I upgraded to the 3.9.0.
I'm very excited about the ability to sign windows builds on osx, but there appears to be a critical bug.

Building windows binaries now takes significantly longer.

The culprit here is mono-sgen.

The process performs the various actions that it must for about thirty seconds, and then mono-sgen kicks in at 99% CPU usage for about ten minutes.

If you could take a look at soon that would be much appreciated.

@develar
Copy link
Member

develar commented Apr 22, 2016

To fix it, we should get rid of Squirrel.windows original exe and rewrite it to some normal, non-***-windows platform.

This work is started — we don't use *** nuget to pack anymore.

I guess, it is a new Squirrel.windows 1.3.0 issue — so, please, file issue to https://github.com/Squirrel/Squirrel.Windows/issues

Also, please attach debug log here — set env DEBUG=electron-windows-installer to get it.

@ArekSredzki
Copy link
Author

ArekSredzki commented Apr 22, 2016

@develar

Here's the log. Sadly it doesn't say much.

Removing /Users/asredzki/tesla/releng/canode/dist/win-x64
Packaging app for platform win32 x64 using electron v0.37.6
  electron-windows-installer Created NuSpec file:
<?xml version="1.0"?>
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">
  <metadata>
    SNIP
  </metadata>
</package> +0ms
  electron-windows-installer Spawning mono /Users/asredzki/tesla/releng/canode/node_modules/electron-winstaller-fixed/vendor/Update-Mono.exe --releasify /Users/asredzki/tesla/releng/canode/dist/win-x64/.tmp/in.nupkg --releaseDir /Users/asredzki/tesla/releng/canode/dist/win-x64 --loadingGif /Users/asredzki/tesla/releng/canode/node_modules/electron-winstaller-fixed/resources/install-spinner.gif --setupIcon /Users/asredzki/tesla/releng/canode/build/icon.ico --no-msi +6s

After the last line of this log appeared (which was just a few seconds into the build), the process ran for about 12 minutes before exiting silently.

@develar
Copy link
Member

develar commented Apr 22, 2016

Please specify your OS, wine --version and mono --version.

@develar
Copy link
Member

develar commented Apr 22, 2016

And please file issue to Squirrel.windows.

@ArekSredzki
Copy link
Author

ArekSredzki commented Apr 22, 2016

OS X version: 10.11.4
Wine version: 1.8.1
Mono version: 4.2.3.4
(Also an issue for Mono 4.2.2.30)

@develar I just discovered that this is not related to the latest release, this occurred because I disabled asar usage.
Is this still something that should be filed with Squirrel.windows?

@ArekSredzki
Copy link
Author

ArekSredzki commented Apr 23, 2016

FYI code signing does not change whether the build time is painfully long.

~10 mins per windows arch on an OS X machine with 3.1GHz i7 & 16gb of ram.

@ArekSredzki ArekSredzki changed the title High CPU usage from mono-sgen in new version High CPU usage from mono-sgen building windows without asar on OS X Apr 23, 2016
@develar
Copy link
Member

develar commented Apr 23, 2016

@ArekSredzki If you disabled ASAR and mono hangs — it is well known issue. Please see electron/windows-installer#55

I am sorry, but unlikely it will be fixed soon. It is a Squirrel.windows bug and Squirrel.windows written in C# and requires windows. So, it is very hard to fix it (requires a lot of time to learn new unpleasant platform and it is a *** windows). Please file issue to Squirrel.windows.

Why you have to disable ASAR? I can fix some issues in this area.

@develar
Copy link
Member

develar commented Apr 23, 2016

Please attache here file node_modules/electron-winstaller-fixed/vendor/SquirrelSetup.log

@ArekSredzki
Copy link
Author

ArekSredzki commented Apr 24, 2016

@develar Thank you for the support, it is much appreciated.

I am definitely interesting in supporting ASAR. Your continued support in doing so would be 👍
The reason that I am "unable" to use it is that I create an additional process using child_process.fork from main that is used for long-running CPU-bound tasks. Unfortunately, I have not been able to get this to work thus far.

Debugging the issue was very painful because the process clearly would not start but also didn't emit an error event (even though the documentation states that it should.

I tried to make use of asar-unpack but the effect for the developer is poorly documented.
That paired with the inability to debug led me to abandon the effort.

Something of interest was that when I unpacked the application I didn't see anything named app.asar.unpacked like I presumably should have.

Any pointers are appreciated.

Also, here is the contents of SquirrelSetup.log after a build:

2016-04-23 19:16:46> Program: Starting Squirrel Updater: --releasify /Users/asredzki/tesla/releng/canode/dist/win-x64/.tmp/in.nupkg --releaseDir /Users/asredzki/tesla/releng/canode/dist/win-x64 --loadingGif /Users/asredzki/tesla/releng/canode/node_modules/electron-winstaller-fixed/resources/install-spinner.gif --setupIcon /Users/asredzki/tesla/releng/canode/build/icon.ico --no-msi
2016-04-23 19:16:46> Program: Bootstrapper EXE found at:/Users/asredzki/tesla/releng/canode/node_modules/electron-winstaller-fixed/vendor/Setup.exe
2016-04-23 19:16:46> Program: Creating release package: /Users/asredzki/tesla/releng/canode/dist/win-x64/in.nupkg
2016-04-23 19:22:21> ReleasePackage: Creating release package: /Users/asredzki/tesla/releng/canode/dist/win-x64/in.nupkg => /Users/asredzki/tesla/releng/canode/dist/win-x64/canode-0.2.0-full.nupkg
2016-04-23 19:22:24> ReleasePackage: Extracting dependent packages: []
2016-04-23 19:22:24> ReleasePackage: Removing unnecessary data
2016-04-23 19:22:24> ReleasePackage: No release notes found in /Users/asredzki/.local/share/SquirrelTemp/tempb/canode.nuspec
2016-04-23 19:22:39> Program: Building embedded zip file for Setup.exe

@develar
Copy link
Member

develar commented Apr 24, 2016

So, I see, it is easy to fix. I will try (use 7z to zip as solution).

asar-unpack — I will check.

@ArekSredzki
Copy link
Author

@develar Care to elaborate about 7z?

@ArekSredzki
Copy link
Author

Are you referring to using .zip instead of .nupkg?
If so, how would that work for Squirrel.windows? Unless of course that is modified.

@develar
Copy link
Member

develar commented Apr 24, 2016

Are you referring to using .zip instead of .nupkg?

nupkg is a plain zip file. What I want to do — Squirrel.windows uses some zip lib to pack (Building embedded zip file for Setup.exe). As a solution, we can try to fix it and use 7z. In any case we should use 7z because 7z provides the best zip compression.

@ArekSredzki
Copy link
Author

ArekSredzki commented Apr 24, 2016

@develar Oh excellent!

I had assumed that .nupkg was fancier than that 😄

Out of curiousity, did this commit relate to this issue or something else to do with nuget?

@develar
Copy link
Member

develar commented Apr 24, 2016

Out of curiousity, did this commit relate to this issue or something else to do with nuget?

Yes. 4x-8x speed up nuget pack :) Ideally, we should get rid of Squirrel.Windows and implement compatible releasify tool.

@ArekSredzki
Copy link
Author

@develar I'm confused then. Why am I still experiencing the slow compression? I'm on 3.9.0

And why replace it rather than fix it?

@develar
Copy link
Member

develar commented Apr 24, 2016

Why am I still experiencing the slow compression?

Because fix was not complete. nuget part was only fixed (it seems, the same zip issue — because nuget uses the same/simular "broken" (?) zip lib).

And why replace it rather than fix it?

Because nuget is a junk. Squirrel.Windows will be also fixed in this way — we will implement zip on our side in a cross-platform way and pass result to Squirrel.Windows (i.e. I am not going to fix it properly on Squirrel.Windows side because it is not possible — Squirrel.Windows written in C#).

@ArekSredzki
Copy link
Author

ArekSredzki commented Apr 25, 2016

@develar Thank you for the further information. Excited to make use of it.

Any hints on how to get child_process.fork working with asar?

Note: I had previously referenced spawn when I actually meant fork

@develar
Copy link
Member

develar commented Apr 26, 2016

Any hints on how to get child_process.fork working with asar?

It is not possible, so, you need to use asar-unpack. I am going to check your statement "but the effect for the developer is poorly documented".

Please specify path that you tried to execute — in the .bin? In this case electron-builder can automatically don't pack such files into asar.

FYI: this issue is definitely will be fixed, because I hate Squirrel.Windows pack and it makes our CI win tests slow and unreliable. Don't expect fix in a few days, but this/next week may be.

@ArekSredzki
Copy link
Author

@develar Thanks. Having unpacked the folder in which the file exists, does one have to add app.asar.unpacked to the path?

The module that I am trying to fork is not in .bin but rather in in my app folder.

@develar
Copy link
Member

develar commented May 11, 2016

asar-unpack — see #390

this issue is progress, but slow (step by step, to not break anything).

@develar
Copy link
Member

develar commented May 13, 2016

@ArekSredzki Mostly fixed in the 3.21.0, but problematic code still in use. Will be finally addressed soon.

@ArekSredzki
Copy link
Author

Thanks @develar I appreciate it!

develar added a commit to develar/electron-builder that referenced this issue May 14, 2016
develar added a commit to develar/electron-builder that referenced this issue May 14, 2016
develar added a commit to develar/electron-builder that referenced this issue May 14, 2016
develar added a commit that referenced this issue May 14, 2016
@develar develar closed this as completed May 14, 2016
@ArekSredzki
Copy link
Author

👍🏼👍🏼👍🏼👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants