Skip to content

Conversation

@exarkun
Copy link
Contributor

@exarkun exarkun commented Dec 3, 2021

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
Copy link
Member

crwood commented Dec 8, 2021

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
Copy link
Contributor Author

exarkun commented Dec 8, 2021

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
Copy link
Member

crwood commented Dec 8, 2021

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