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

Add tracing tools and documentation #898

Merged
merged 12 commits into from
Aug 14, 2023
Merged

Add tracing tools and documentation #898

merged 12 commits into from
Aug 14, 2023

Conversation

caizixian
Copy link
Member

@caizixian caizixian commented Aug 11, 2023

These tools utilize the tracepoints added in #883. The GC visualization tools requires some post-processing and is just a bit more complicated in general. That will be added in a separate PR.

These tools utilize the tracepoints added in mmtk#883

Co-authored-by: Claire Huang <claire.x.huang@gmail.com>
@wks
Copy link
Collaborator

wks commented Aug 11, 2023

Directory structure: I think this tracing tool will not be the only tool we provide. I recommend adding one level of sub-directory, such as mmtk-core/tools/tracing/

tools/run.py Outdated Show resolved Hide resolved
tools/run.py Outdated Show resolved Hide resolved
tools/run.py Outdated
Comment on lines 45 to 53
with tempfile.NamedTemporaryFile(mode="w+t") as tmp:
content = template.safe_substitute(
MMTK=mmtk_bin, TMP_FILE=tmp.name)
if args.print_script:
print(content)
tmp.write(content)
tmp.flush()
os.execvp("sudo", ["sudo", args.bpftrace,
"--unsafe", "-f", args.format, tmp.name])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python will automatically delete the NamedTemporaryFile when exiting the scope of with. However, execvp never returns. You can replace this with subprocess.run or subprocess.Popen.

An alternative to using temp file is using pipes. I tried piping the script into sudo bpftrace and it works. For example, I am currently using erb (from Ruby) for templating, and my command line looks like this:

erb record_every=50 so_path=/home/wks/projects/mmtk-github/openjdk/build/linux-x86_64-normal-server-release/jdk/lib/server/libmmtk_openjdk.so  gcvis.erb | sudo bpftrace - > openjdk-packet-size3-nopoll.log

In Python, the subprocess.run function (or popen.communicate) allows you to specify the stdin as a pipe, so you can directly pipe the Python string prologue + content + epilogue into sudo bpftrace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a deliberate choice. I will add comments to explain this.

Comment on lines 1 to 3
END {
system("rm $TMP_FILE");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to delete the temp file if we use exec. See the comment on run.py.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

tools/README.md Outdated Show resolved Hide resolved
tools/README.md Outdated Show resolved Hide resolved
tools/README.md Outdated
Comment on lines 81 to 83
### Measuring the time spend in different GC stages (`gc_stages`)
This tool measures the time spent in different stages of GC: before `Closure`, during `Closure`, and after `Closure`.
The time unit is ns.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A general style problem: I find that you never uses newline characters within one paragraph. Markdown allows this.

But here the text spans over three lines. I think you intend to introduce one title and two paragraphs. However, Markdown will interpret consecutive non-blank lines as one paragraph (except for titles (### blah blah blah)). If you intend to put "The time unit is ns." in a single paragraph, you need to add an empty line before it.

I also recommend formatting the text so that the line width is no more than 100 characters long (or 80 or 72 if you prefer). The fmt command line utility may come in handy. If you are a Vim user, you can set tw=100, select one or more paragraphs and press gq to format them. In this way, this text file will still be easy to read using text editors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

tools/README.md Outdated
Comment on lines 133 to 134
In the above output, we can see that the lock instance (140637228007056, or 0x7fe8a8047e90) roughly has a bimodal distribution in terms of the time spent in lock contended code path. The first peak is around 512ns\~1024ns, and the second peak is around 66us\~131us.
If you can't tell which lock instance is for which lock in MMTk, you can trace the allocation of the Mutex and record the stack trace (note that you might want to compile MMTk with `force-frame-pointers` to obtain better stack traces).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they are two paragraphs, there needs to be an empty line in between.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@caizixian caizixian requested a review from wks August 11, 2023 10:20
@caizixian
Copy link
Member Author

Directory structure: I think this tracing tool will not be the only tool we provide. I recommend adding one level of sub-directory, such as mmtk-core/tools/tracing/

Yeah, that makes sense. Fixed.

@caizixian caizixian added A-utils Area: Utilities G-performance Goal: Performance C-feature Category: Feature A-benchmarking Area: Benchmarking MMTk labels Aug 12, 2023
@steveblackburn
Copy link
Contributor

I am confused about the time "units". Do you really mean units or do you mean resolution? The text seems to use seconds (or us / ns) as the units.

Other than that, this looks excellent. Nicely documented and packaged.

@caizixian
Copy link
Member Author

caizixian commented Aug 12, 2023

I am confused about the time "units". Do you really mean units or do you mean resolution? The text seems to use seconds (or us / ns) as the units.

Other than that, this looks excellent. Nicely documented and packaged.

@steveblackburn Yes I meant units. Just to clarify, it is about how much one time unit is in the tracing output. We sometimes scale the numbers to have better resolution on the log2-based histogram, where the numbers on the histogram bins are in the time unit. This is different from the measurement resolution.

Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few more things to fix.

tools/tracing/run.py Outdated Show resolved Hide resolved
tools/tracing/lock_contended.bt Outdated Show resolved Hide resolved
tools/tracing/lock_contended.bt Outdated Show resolved Hide resolved
caizixian and others added 4 commits August 14, 2023 16:43
Co-authored-by: Kunshan Wang <wks1986@gmail.com>
Co-authored-by: Kunshan Wang <wks1986@gmail.com>
Co-authored-by: Kunshan Wang <wks1986@gmail.com>
Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@caizixian caizixian merged commit b6ffcdd into mmtk:master Aug 14, 2023
17 checks passed
@caizixian caizixian deleted the tools branch August 14, 2023 12:47
wenyuzhao pushed a commit to wenyuzhao/mmtk-core that referenced this pull request Aug 17, 2023
These tools utilize the tracepoints added in mmtk#883. The GC visualization
tools requires some post-processing and is just a bit more complicated
in general. That will be added in a separate PR.

---------

Co-authored-by: Claire Huang <claire.x.huang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-benchmarking Area: Benchmarking MMTk A-utils Area: Utilities C-feature Category: Feature G-performance Goal: Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants