Simplify the use of emprofile and fix issue with empty generated profiles. #13464
Conversation
|
CI is going sideways on unrelated change: |
|
Updated this PR to rename |
juj
force-pushed
the
improve_emprofile
branch
from
74bef23 to
65c618f
Compare
|
Ping, this could use a review. |
juj
force-pushed
the
improve_emprofile
branch
from
65c618f to
2b01b2d
Compare
…E", so that users only need to remember one word "EMPROFILE" to use the toolchain profiler.
juj
force-pushed
the
improve_emprofile
branch
from
ed80112 to
5ea463e
Compare
|
@sbc100 you recently worked on this, I think, maybe you want to take a look? If you don't have time I can try to. |
| cd path/to/emscripten | ||
| tools/emprofile.py --reset | ||
| export EM_PROFILE_TOOLCHAIN=1 | ||
| export EMPROFILE=1 |
There was a problem hiding this comment.
Why change the name of this environment variable?
I myself only know of one user out there but there maybe be others.
| @@ -0,0 +1,29 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
You can create these while keeping the tool under tool if you like?
You can just add tools/embuilder to tools/create_entry_points.py ad then re-run it.
These scripts will then get created in the tools directory, which I think it probably fine.
If you really want to make it so that folks don't have to type ./tools/empofile but just ./emprofile I think there are a few other places in the docs that will need updating.
I think it might be simple to leave it there myself but I don't have strong feelings.
There was a problem hiding this comment.
I do want the users to be able to run just emprofile and not $EMSCRIPTEN/tools/emprofile, the earlier pattern to enable was too complex, and I had to always look at the doc page after a few months had passed since I used this tool. Now after this enabling the profiler should be much simpler.
There was a problem hiding this comment.
OK sounds reasonable. Perhaps we need to modify create_entry_points.py to handle creating wrappers at the top level to tools in the tools directory. I can take a look at this if you like?
There was a problem hiding this comment.
Actually it looks like this would make the bat/sh run_python scripts way less trivial. Would you mind if we land this change without moving emprofille out of the tools directory.
We can then followup with a plan to enable tools like emdump emprofile and file_packager to be moved to the top level at some point in the future? It seems like we should have a generic solution here.
lgtm with create_entry_points.py usage and keeping launchers under tools. (for now)
There was a problem hiding this comment.
I have that other PR for ccache that already rewrites parts of the scripts creation, I'd prefer not to have to manage conflicts here. Hoping this could land as-is..?
There was a problem hiding this comment.
I took a look at solution for the create_entry_points.py problem: #13679
Hopefully it will make this change easier.
As and example the python code for the emdump tool lives in tools/emdump.py, but we want the entey point to be at the top level of emscripten. Hopefully this will help with #13464
As and example the python code for the emdump tool lives in tools/emdump.py, but we want the entey point to be at the top level of emscripten. Hopefully this will help with #13464
|
I'll land this as-is, we can iterate on the format later. |
As and example the python code for the emdump tool lives in tools/emdump.py, but we want the entey point to be at the top level of emscripten. Hopefully this will help with #13464
As and example the python code for the emdump tool lives in tools/emdump.py, but we want the entey point to be at the top level of emscripten. Hopefully this will help with #13464
As and example the python code for the emdump tool lives in tools/emdump.py, but we want the entey point to be at the top level of emscripten. Hopefully this will help with #13464
3 participants
Simplify the use of emprofile and fix issue with empty generated profiles.
Add top level
emprofileshortcut to avoid needing to use awkwardpython $EMSCRIPTEN/tools/emprofile.pystring on command line.