On Sat, Jan 25, 2014 at 2:36 PM, Victor Stinner <[email protected]>wrote:
> 2014-01-25 Guido van Rossum <[email protected]>: > >> It looks like Charles-François doesn't like it: > >> http://bugs.python.org/issue20311#msg209213 > > > > Hm. > > > >> My latest patch is more generic because it uses also the resolution of > >> the clock. > > > > That sounds right. > > > > However, now that I see the code, I have a few questions. (Sorry for > > shooting first and asking questions later.) > > > > - Is it intentional that granularity is a public attribute of the event > > loop? (I can see a use case but also abuses and confusion.) > > - Is it intentional that granularity can be *set* by users of the event > > loop? > > Ah, it was one point in my TODO list, I forgot to mention it :) > > I see two use cases: > > - Override the time() method in an event loop: you must be able to > modify the granularity using the new clock resolution. (Same point for > a custom selector.) > Subclassing event loop implementations is not recommended. There are too many things that might break when the base class evolves. > - On an embedded device, you may prefer to limit power compution > instead of providing the best reactivity. For an user interface, a > granularity of 10 ms may be enough, no need to react in 1 nanosecond. > If we ever want to support that use case there should probably be different API for it. > I'm not sure that the second use case is related to what I wrote in > _run_once(). Rounding the deadline (away from zero) should maybe be > done in call_later() instead. > > I'm fine with a read-only property using a private _granularity attribute. > Just make it a private _granularity attribute (like everything else in the event loop classes) -- I don't see a need for a read-only property. > > - Personally I would write the code differently -- I would not change the > > timeout passed to the selector at all, and when going over the _scheduled > > list I would not use ceil() but write it like this: > > > > now = self.time() > > while self._scheduled: > > handle = self._scheduled[0] > > if handle._when > now + self.granularity: > > break > > handle = heapq.heappop(self._scheduled) > > self._ready.append(handle) > > I tested: the unit test pass. I prefer your version because it is > simpler (it doesn't need the math module). See attached patch. Feel > free to commit it, or tell if I can push it. > Please push it. (Aside: these days I find that almost all places where people are inclined to use math.floor() or .ceil() there's a code smell, and there's almost always a better way.) -- --Guido van Rossum (python.org/~guido)
