Skip to content

Fix the await_ready polling rate#415

Merged
crwood merged 10 commits into
gridsync:masterfrom
exarkun:414.await_ready-rate-limit
Dec 8, 2021
Merged

Fix the await_ready polling rate#415
crwood merged 10 commits into
gridsync:masterfrom
exarkun:414.await_ready-rate-limit

Conversation

@exarkun

@exarkun exarkun commented Dec 3, 2021

Copy link
Copy Markdown
Contributor

Fixes #414

This brings CPU usage all the way down to 30% - comparable to where pulseaudio or Firefox idles :( I guess that's what passes for industry standard now.

Maybe at some future point we can actually get rid of polling to address this the rest of the way.

@crwood

crwood commented Dec 8, 2021

Copy link
Copy Markdown
Member

Thanks a lot for this clever solution. Even as a temporary fix, this is a big improvement over the previous behavior (and I got to learn about MemoryReactorClock.pump -- cool!).

One thing I noticed here (and please correct me if I'm misunderstanding) is that the result of Poller.target isn't actually being propagated up to the waiting Deferred (and, likewise, your Failure isn't wrapping the exception) resulting in those details being swallowed. Given the currently-very-narrow/singular usage of Poll, however, this isn't a big deal -- and is certainly not worth holding anything up.

Merging now... Thanks again!

@crwood crwood merged commit de34691 into gridsync:master Dec 8, 2021
@exarkun

exarkun commented Dec 8, 2021

Copy link
Copy Markdown
Contributor Author

Thanks a lot for this clever solution. Even as a temporary fix, this is a big improvement over the previous behavior (and I got to learn about MemoryReactorClock.pump -- cool!).

One thing I noticed here (and please correct me if I'm misunderstanding) is that the result of Poller.target isn't actually being propagated up to the waiting Deferred (and, likewise, your Failure isn't wrapping the exception) resulting in those details being swallowed.

Hm. I didn't test that case so we should assume it is broken. :) However, considering this code from Poller:

        try:
            ready = yield self.target()
            if ready:
                self._completed()
            else:
                self._schedule()
        except Exception:  # pylint: disable=broad-except
            self._deliver_result(Failure())

that propagation is exactly the intent. If self.target() fails then the except Exception: block should execute. Failure() (called with no arguments like that) captures the "current exception state" to initialize the Failure instance. Then _deliver_result will send it to all waiting Deferreds. It does so with Deferred.callback but that is equivalent to Deferred.errback when called with a Failure.

There's plenty of places things could go wrong there so I should have tested it...

@exarkun exarkun deleted the 414.await_ready-rate-limit branch December 8, 2021 20:42
@crwood

crwood commented Dec 8, 2021

Copy link
Copy Markdown
Member

Failure() (called with no arguments like that) captures the "current exception state" to initialize the Failure instance.

Ooh... I did not know that a Failure() would bubble everything up like that (and assumed that, given the python mantra of "explicit is better than implicit", the outer exception object would naturally have to be passed in). Then again, I typically use inlineCallbacks with traditional exceptions over the callback/errback methods... Anyway, good to know! Thank you for explaining. :)

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.

GridSync drives Tahoe at 100% CPU usage if Tahoe can't connect to enough storage servers

2 participants