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

Support running Speedometer 3.0 benchmarks on Chrome #147

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shamouda
Copy link
Contributor

This version supports running Speedometer 3.0 benchmarks located at https://github.com/WebKit/Speedometer on Chrome.

There are two hacks in this change, which can possibly be done better:

  1. I didn't find yet an easy way for running-ng terminate Chrome automatically after the benchmark iterations complete. As a workaround, I relied on timeouts to kill Chrome, and used benchmark.is_passed() in minheap.py and runbms.py to figure out if the timeout should be regarded as a success or failure. This leads benchmarks to take longer to complete, because one needs to configure a long enough timeout that guarantees completing the N iterations.

  2. I didn't know what is the best way to enforce deleting Chrome's cache between iterations. At the moment the hack I implemented is by adding rm -rf /tmp/chrome-expr-cache; in get_log_prologue, and configuring the benchmark to use the same cache directory.

This version supports running Speedometer 3.0 benchmarks located at
https://github.com/WebKit/Speedometer on Chrome.

There are two hacks in this change, which can possibly be done better:

1) I didn't find yet an easy way for running-ng terminate Chrome automatically
after the benchmark iterations complete. As a workaround, I relied on timeouts
to kill Chrome, and used benchmark.is_passed() in minheap.py and runbms.py to
figure out if the timeout should be regarded as a success or failure.
This leads benchmarks to take longer to complete, because one needs to configure
a long enough timeout that guarantees completing the N iterations.

2) I didn't know what is the best way to enforce deleting Chrome's cache between
iterations. At the moment the hack I implemented is by adding
`rm -rf /tmp/chrome-expr-cache;` in get_log_prologue, and configuring the
benchmark to use the same cache directory.
s = ''
for j in self.js_args:
s += j + " "
cmd.append("--js-flags={}".format(s))
Copy link
Member

Choose a reason for hiding this comment

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

The above can be simplified to cmd.append("--js-flags={}".format(" ".join(self.js_args)))

@@ -196,6 +196,8 @@ def hz_to_ghz(hzstr: str) -> str:
def get_log_prologue(runtime: Runtime, bm: Benchmark) -> str:
output = "\n-----\n"
output += "mkdir -p PLOTTY_WORKAROUND; timedrun; "
# Chrome workaround to remove the cache directory between iterations.
output += "rm -rf /tmp/chrome-expr-cache; "
Copy link
Member

@caizixian caizixian Mar 17, 2024

Choose a reason for hiding this comment

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

This doesn't actually run anything. It gets added to the log file, but that's it. You want to use system to execute commands.

Generally, you probably don't want to do this for all runs (which is the case if it gets put inside get_log_prologue), and probably should avoid hardcoded paths (tempfile.TemporaryDirectory) would be a better choice.

Copy link
Member

Choose a reason for hiding this comment

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

Note that running has already created a temporary directory for each run.

https://github.com/anupli/running-ng/blob/815cf54d96b3b50e9c1b3001e63dd6292bd51373/src/running/command/runbms.py#L391C59-L391C69

So you might be able to create a subfolder under it as the temporary directory for Chrome.

Copy link
Member

Choose a reason for hiding this comment

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

So what might be the cleanest is to use the plugin API to create the temporary folder under runbms_dir for Chrome to use. The plugin object will be created by an instance of Chrome, and will only run when Chrome is used for example.

The plugin object can also delete the folder when the benchmark finishes.

Copy link
Member

@caizixian caizixian left a comment

Choose a reason for hiding this comment

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

As you said, there are two general problems.

  1. There are some additional setup/teardown required.
  2. Unlike other existing benchmarks, the VM doesn't exit when Speedometer finishes.

1 is easier to solve. You can register a plugin https://github.com/anupli/running-ng/blob/815cf54d96b3b50e9c1b3001e63dd6292bd51373/src/running/command/runbms.py#L459C13-L459C20 to do the setup/cleanup. The plugin API is here https://github.com/anupli/running-ng/blob/815cf54d96b3b50e9c1b3001e63dd6292bd51373/src/running/plugin/runbms/__init__.py
This is similar to how we reuse the modifier API to set heap sizes, which is cleaner than some special case code just to set heap sizes.

2 is trickier. Is it possible for this to be implemented on the VM side so that you can patch the benchmark to call a VM intrinsic (which might not exist) to shutdown the VM?

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