-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Reorganize the debugging documentation #25018
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
Conversation
@kripken I'm a bit confused about the doc building failure (e.g. https://app.circleci.com/jobs/github/emscripten-core/emscripten/1000395). Do I need to update emcc.txt manually, or is that expected to be done by the CI? |
Oh, actually I think the answer must be "yes" (do it locally), but unfortunately it seems my local configuration must be slightly different than the one on CI because I'm getting different kinds of quote characters :/ |
Weird about those quotes... maybe a different sphinx version? |
Yeah... I don't think it is? I initially tried with whatever version was on my system and it complained. I installed everything into a pip env, and it worked then. So I'm not sure what's going on. |
tools/link.py
Outdated
@@ -1878,7 +1878,7 @@ def get_full_import_name(name): | |||
settings.PRE_JS_FILES = options.pre_js | |||
settings.POST_JS_FILES = options.post_js | |||
|
|||
settings.MINIFY_WHITESPACE = settings.OPT_LEVEL >= 2 and settings.DEBUG_LEVEL == 0 and not options.no_minify | |||
settings.MINIFY_WHITESPACE = settings.OPT_LEVEL >= 2 and settings.DEBUG_LEVEL == 0 and not options.no_minify #shouldn't no_minify be equivalent to g1? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, no it was more of a note-to-self when I found something surprising, I think I just intended to ask you about it. so... is minify=0 supposed to be different to what g1 does? I don't completely understand the implementation and AFAIK we don't have any tests that can tell the difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they should be the same. The docs say
"--minify 0"
[same as -g1 if passed at compile time, otherwise applies at link]
Identical to "-g1".
and I can't think of a difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I think the thing I saw was there were places that DEBUG_LEVEL is referred to where no_minify wasn't, and thought maybe this can be simplified (e.g. removing settings.no_minify and just making it equivalent to g1), but that's not for this PR in any case.
Co-authored-by: Alon Zakai <[email protected]>
|
||
- | ||
.. _emcc-g3: | ||
|
||
``-g3``: When compiling to object files, keep debug info, including JS whitespace, function names, and LLVM debug info (DWARF) if any (this is the same as :ref:`-g <emcc-g>`). | ||
``-g3``: Also keep LLVM debug info (DWARF) if there is any in the object files (this is the same as :ref:`-g <emcc-g>`). | ||
|
||
.. _emcc-profiling: | ||
|
||
``--profiling`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it is preexisting, but it's not clear to me what this does.. (I also haven't used this myself)
It says it's similar to -g2
, then what's the difference?
Also the name is similar to that of --profiling-funcs
; are they related in any way? Is one is a subset of the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe --profiling-funcs
is supposed to be a subset of --profiling
yes.
I'd love to get rid of these two flags and replace --profiling-funcs
with something like -gname-section
which is basically what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--profiling
does all the optimizations that would affect speed (to make sure the profile execution is the same as the production one) but keeps function names in wasm and JS (i.e. no minification), so that the profile is actually useful. (--profiling-funcs
enables the name section in the wasm binary but still minifies JS).
I'm not sure about the history of these profiling flags vs -g2
. Currently AFAIK they are the same. In an earlier version of this PR I posed a similar question (see above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rewrote the description here to make it more accurate and clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbc100 regarding -gnames, see the discussion on #20462. We have -g2 (which is names+JS whitespace). If you want names only, I guess you can do -g2 --minify=0
. I think it's an open question whether we want to make it easier still to get names only, and what the use case is. I can see how you might want names-only (or names + source maps but not minification) if you want full production build with function names and locations. But maybe we'll also have #25044 for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it looks we have too many flags that are subtly different from each other... It'd be nice if we can get rid of some of them at least..
test/test_other.py
Outdated
@@ -9261,6 +9262,9 @@ def test_binaryen_debug(self): | |||
(['-O2', '-g'], False, True, False), | |||
(['-O2', '--closure=1'], True, False, True), | |||
(['-O2', '--closure=1', '-g1'], True, True, True), | |||
(['-O2', '--minify=0'], False, True, False), | |||
(['-O2', '--profiling-funcs'], True, False, False), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's "cleaned" JS and which tool does it? Reading the test suggests it lacks comments. Is that the only difference from the "normal" JS?
And do we have info on the JS optimizations? This test suggests we have at least three kinds of JS optimizations (minification, cleaning, and closure)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, what I know is basically what these test actually check for, i.e. comments (and presumably also optimizations?) vs whitespace. I assume that clean-but-with-whitespace is what you want for profiling, since you want the speed optimizations that the final product would have, but want useful function names in your profile and stack traces and readability. But I'm not sure which tool does what and where. Probably @kripken knows.
@kripken @sbc100 the doc building step is failing here (https://app.circleci.com/pipelines/github/emscripten-core/emscripten/45613/workflows/3565df1d-01e6-4365-a540-0be846a3a4c3/jobs/1022087) because apparently when I run it locally I'm getting fancy quote characters and different dashes in some cases (when generating emcc.txt). Any idea what might be going on here? |
Do you have the exact version of sphinx installed from |
Yes. but nevermind I blew away the build directory and reran the build in my pip env and it worked again. |
But since I've now @-ed you all... let's do the thing where we bikeshed how many columns I should wrap all this text to :) |
I am always in favor of more columns but I always lose the debate 😄 I don't feel strongly. |
Yeah I had been thinking that we settled on something larger than 80 but more than "ALL the columns" but maybe I'm misremembering. |
Since basically the whole file is touched in this PR anyway, I just re-wrapped the whole thing. |
LGTM but it'd be nice to have some documentation on JS optimizations too (as a followup, when you have time): #25018 (comment) |
Most of the existing text is the same, but reorganized around use cases, with examples.
Incorporates some content from #20462