Add syscall count benchmark for 002_hello.ts #820
Conversation
tools/benchmark.py
Outdated
| lambda line: "<... unfinished>" not in line and # <unfinished> and <resume> repeats same syscall | ||
| "+++ exited with" not in line, # lines labelling exit | ||
| [deno_path, "tests/002_hello.ts", "--reload"]) | ||
| return syscall_count_map |
There was a problem hiding this comment.
Since the parsing of strace is becoming more complex, I think you should store a trace
strace deno tests/001_hello.js >& tools/testdata/strace1.txt
And then check that you get the correct value in benchmark_test.py. This will be much more robust and debuggable than just testing > 1.
There was a problem hiding this comment.
Just noticed that strace has an option -c that generates a nice call summary. However, summaries show the same program calling syscalls different times and with slightly different orders. For example, in my testing, futex is called with either 3 or 4 times and varies across runs. Dunno if same program would even have a slightly different set of syscalls running each time (I suspect that certain packages might actually do such things)
There was a problem hiding this comment.
The syscalls are definitely not going to be constant - there's a lot of complexity in deno. Using the -c option sounds great - store the output into //tools/testdata/strace_summary.txt and check that you can parse it independently of the benchmark.
There was a problem hiding this comment.
Just want to clarify, the purpose of this file is to ensure that we can do parsing correctly, right? For example, if we have a table saved in strace_summary.txt that looks like
calls name
=============
4 futex
2 mmap
then would validate in our benchmark test that the our parsing logic would give us exactly 6 for total syscall count. Am I understanding correctly?
There was a problem hiding this comment.
Yes
let r = strace_parse("tools/testdata/systrace_count.txt");
assert r['futex'] == 4
assert r['mmap'] == 2
kevinkassimo
force-pushed
the
benchmark/syscall_count
branch
from
0851f92
to
183986c
Compare
tools/benchmark.py
Outdated
| try: | ||
| http_server.spawn() | ||
| except: | ||
| "Warning: another http_server instance is running" | ||
|
|
||
| # The list of the tuples of the benchmark name and arguments | ||
| benchmarks = [("hello", ["tests/002_hello.ts", "--reload"]), |
There was a problem hiding this comment.
I know this isn't part of your commit, but could you rename benchmarks to exec_time_benchmarks ?
There was a problem hiding this comment.
LGTM!
I've been reloading updates of https://denoland.github.io/deno/ all day : )
Uses
strace. Specially handling interleaving syscall report cases (<unfinished>and<resumed>, keep only one of them) and skipping+++ exited with 0 +++lines