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

Benchmark results never gets reported #202

Closed
FFY00 opened this issue Sep 18, 2020 · 3 comments
Closed

Benchmark results never gets reported #202

FFY00 opened this issue Sep 18, 2020 · 3 comments
Labels
software Component: software

Comments

@FFY00
Copy link
Contributor

FFY00 commented Sep 18, 2020

$ glasgow run benchmark source
I: g.device.hardware: device already has bitstream ID e00c76bf7d43e9c1b138500bf7df1a1f
I: g.cli: running handler for applet 'benchmark'
I: g.applet.internal.benchmark: running benchmark mode source for 8.000 MiB

I've traced the issue to iface.read never continuing and bisected the breaking patch to a7d041f.

@FFY00
Copy link
Contributor Author

FFY00 commented Sep 18, 2020

Looks like that commit can be cleanly reverted, and after that everything does work as supposed to.

$ glasgow run benchmark source
I: g.device.hardware: device already has bitstream ID e00c76bf7d43e9c1b138500bf7df1a1f
I: g.cli: running handler for applet 'benchmark'
I: g.applet.internal.benchmark: running benchmark mode source for 8.000 MiB
I: g.applet.internal.benchmark: mode source: 25.11 MiB/s (200.85 Mb/s)

@FFY00
Copy link
Contributor Author

FFY00 commented Sep 18, 2020

The following patch solves the issue.

diff --git a/software/glasgow/support/task_queue.py b/software/glasgow/support/task_queue.py
index 25cda9f..cb87c0c 100644
--- a/software/glasgow/support/task_queue.py
+++ b/software/glasgow/support/task_queue.py
@@ -21,7 +21,8 @@ class TaskQueue:

     def _callback(self, future):
         self._live.remove(future)
-        self._done.append(future)
+        if not future.cancelled():
+            self._done.append(future)

     def submit(self, coro):
         """

I miss the background behind this change, @whitequark could you clarify?

@whitequark
Copy link
Member

The background for the change is described in the commit message. Whether your patch is correct or not, I can't say offhand because asyncio cancellation semantics is annoyingly tricky.

@whitequark whitequark added benchmark Applet: benchmark software Component: software and removed benchmark Applet: benchmark labels Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
software Component: software
Projects
None yet
Development

No branches or pull requests

2 participants