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

Responsive logging #1631

Merged
merged 2 commits into from Sep 26, 2013
Merged

Responsive logging #1631

merged 2 commits into from Sep 26, 2013

Conversation

juj
Copy link
Collaborator

@juj juj commented Sep 18, 2013

Mostly Windows-specific things:

  • Make the Windows Python Popen workaround disabled by default. Fortunately it does not seem to be needed when emscripten is invoked from the command line via the emcc.bat scripts. Keeping it around behind an .emscripten param/env var EM_POPEN_WORKAROUND in case it is still needed.
  • Unify logging in emscripten.py behind a single function to force flush all logging immediately to console. On Windows, building large codebases would look like they hang, since the above Popen workaround and delayed logging made the build output appear only after the externally spawned processes finish.
  • Add machinery to external process spawns and waits with EM_BUILD_VERBOSE >= 3 to be able to debug where the build hangs, if it hangs.

@kripken
Copy link
Member

kripken commented Sep 18, 2013

I worry that flush all the time can be slower on other OSes - perhaps we should only do that if WINDOWS?

@kripken
Copy link
Member

kripken commented Sep 18, 2013

We should likely be using logging.debug here, like elsewhere. Or is there something special in emscripten.py where logging is more of an issue? It should not generate that much actually, the bulk of the logging in most cases is from the processes it spawns (the JS compiler itself).

@juj
Copy link
Collaborator Author

juj commented Sep 18, 2013

Ok, I'll make the flush Windows-only. Can you elaborate on the second point, what do you refer to there? it seems that the comment is not annotated on a specific source line. Do you mean that the implementation of logmsg should do logging.debug() instead of print >> sys.stderr? Or that only the jsrun.py process tracking should do logging.debug() instead of logmsg?

@kripken
Copy link
Member

kripken commented Sep 24, 2013

I meant that we should use logging.* consistently in all places, unless there is a reason to do otherwise.

…he logging framework prints out messages unbuffered which is more responsive on Windows.

Add debug logging facility to track waits on external processes when EM_BUILD_VERBOSE >= 3. This helps pinpointing if the build hangs on some tool dying/live/deadlocking, and where it might occur.

Implement process.pid on WindowsPopen replacement so that EM_BUILD_VERBOSE=3 works on it as well.
… call emcc.bat usage doesn't seem to need it, and it adversely affects the logging buffering that makes the compiler look unresponsive, since it will only print out compilation output messages at the very end of the whole run.
@juj
Copy link
Collaborator Author

juj commented Sep 26, 2013

Ok, I updated this pull request to use logging.* instead of logmsg(). Fortunately logging prints out messages unbuffered, which print didn't seem to do, so the flush doesn't seem even necessary!

kripken added a commit that referenced this pull request Sep 26, 2013
@kripken kripken merged commit ebfb4a9 into emscripten-core:incoming Sep 26, 2013
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.

None yet

2 participants