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

MD5 speed test not working on Mac #149

Open
Roger-Shepherd opened this issue Oct 18, 2021 · 6 comments
Open

MD5 speed test not working on Mac #149

Roger-Shepherd opened this issue Oct 18, 2021 · 6 comments

Comments

@Roger-Shepherd
Copy link
Contributor

Roger-Shepherd commented Oct 18, 2021

Timing reporting mechanism fails to report timing. The log file looks like

Warning: Failed to find timing
Args to subprocess:
sh '-c' './md5sum; echo RET=$?'
cbecbdb0fdd5cec1e242493b6008cc79
... 147899 lines more of the same
cbecbdb0fdd5cec1e242493b6008cc79
Real time: 693.355000 ms CPU time: 693.312000 ms
RET=1

Under investigation; failure happens for x86 Mac and M1 running ARM or x86 code

@Roger-Shepherd
Copy link
Contributor Author

The problem is that the benchmark is writing to std out in benchmark_body. The Mac scripts assumes the only info on standard out is timing info. I think the print statements should be removed, or at least put under a debugging option.

@hirooih
Copy link
Contributor

hirooih commented Oct 20, 2021

Hi,

I took a look at the code.
The following patch should fix this issue.
I don't have Mac and cannot test it by myself. Could you try this?

--- a/pylib/run_mac.py
+++ b/pylib/run_mac.py
@@ -64,7 +64,7 @@ def decode_results(stdout_str, stderr_str):
         return 0.0
 
     # Match "Real time: dd.ddd"
-    time = re.search('^Real time: (\d+)[.](\d+)', stdout_str, re.S)
+    time = re.search('^Real time: (\d+)[.](\d+)', stdout_str, re.M)
     if time:
         ms_elapsed = float(time.group(1) + '.' + time.group(2))
         # Return value cannot be zero (will be interpreted as error)

I think the print statements should be removed, or at least put under a debugging option.

I agree with you. The current md5sum test is measuring the performance of printf() :-)

@Roger-Shepherd
Copy link
Contributor Author

Roger-Shepherd commented Oct 20, 2021 via email

@jeremybennett
Copy link
Collaborator

I think the fix proposed by #152 addresses the use of external library functions.

@hirooih
Copy link
Contributor

hirooih commented Nov 14, 2021

This is what Roger and I wrote.

I think the print statements should be removed, or at least put under a debugging option.
I agree with you. The current md5sum test is measuring the performance of printf() :-)

I still recommend you to replace re.S with re.M in pylib/run_*.py.

https://docs.python.org/3/library/re.html#re.S
https://docs.python.org/3/library/re.html#re.M

@Roger-Shepherd
Copy link
Contributor Author

The change to pylon/run_*.y suggested does allow the benchmark to run.

We still need to remove the print statements though.

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

No branches or pull requests

3 participants