Hi Glyph, On March 22, 2016 at 11:27:10 PM, Glyph (glyph@twistedmatrix.com) wrote: On Mar 22, 2016, at 12:33 PM, L. Daniel Burr <ldanielburr@me.com> wrote: Hi all, Hi Daniel, I tried to comment on the ticket, but SpamBayes rejected it as spam. The spam-monitoring queue is empty, which means another admin probably got to this before I did.  But when I plugged the following paragraph in, something about it made spambayes think it was still 70% likely that it was spam.  So, I re-trained the filter repeatedly until it came out to <1%; you should have less trouble with it in the future. Thanks for looking into that, I appreciate it. As a person who runs twisted apps via Pypy whenever possible, I wanted to point out that this ticket may result in a performance regression: according to http://pypy.org/performance.html, "Classes that inherit from both new- and old-style classes are extremely slow; avoid at all costs." In fact, the opposite is true!  Right now, every new Twisted class must be new-style, so if it inherits from an old-style class we end up in this situation.  Since most core Twisted superclasses are old-style, that means this happens all the time. Nothing about 8244 would involve making more hybrid classes.  Classes decorated as @oldStyle must be pure old style (the semantics of hybrids are much, much closer to new-style than old-style) so they have to be what they are today.  When we flip the switch there will be no more old-style classes at all. Is there a scenario I'm missing / not understanding about the way the ticket's steps are outlined? I’m thinking of a twisted.web service at the moment.  Right now, most of the classes involved (resource.Resource, http.HTTPChannel, http.Request and server.Request, etc.) are old-style classes, and pypy does a reasonable job of optimizing those, if not to the degree that new-style classes may be optimized.  My concern is that, should these classes be converted into hybrids via the @oldStyle decorator, my service may experience some reduction in the number of requests per second that it handles.  Obviously I’m using pypy because it gives my twisted.web service a substantial improvement in terms of the number of requests per second that it can handle. I understand that I can set TWISTED_NEWSTYLE to 0 and sidestep this potential performance hit, but it is a detail that has to be communicated to various devops teams, has to be kept track of across environments, should probably be removed once a fully-converted release of Twisted arrives, and so on. Yes, I’m being a bit of a whiner here (apologies), and clearly the feature can be disabled, and I should definitely test the results to see if there *is* a performance regression in my case. Thanks, Daniel -- L. Daniel Burr ldanielburr@me.com (312) 656-8387

Hi Glyph, On March 23, 2016 at 1:39:53 PM, Glyph (glyph@twistedmatrix.com) wrote: On Mar 23, 2016, at 9:22 AM, L. Daniel Burr <ldanielburr@me.com> wrote: My concern is that, should these classes be converted into hybrids via the @oldStyle decorator, my service may experience some reduction in the number of requests per second that it handles If oldStyle converts the classes into hybrids, then it's broken.  Hybrids don't follow the semantics of old-style classes.  So this won't happen.  I should definitely test the results to see if there *is* a performance regression in my case. My guess is that this is an insignificant micro-optimization and that pypy has gotten better at optimizing hybrids since that caution was written.  That said, we're really bad at paying attention to speed.twistedmatrix.com and we really need a way to see pre-merge performance metrics rather than just a graph over time. In the interest of putting my money where my mouth is, I’ve tested a very simple case, where I create a class inheriting from twisted.web.resource.Resource, and a subclass which mixes in “object”: from twisted.application import internet, service from twisted.internet import endpoints, reactor from twisted.web import resource, server class MyResource(resource.Resource): isLeaf = True def render_GET(self, request): return 'Hello, World!' class FrankenStyle(MyResource, object): pass print type(MyResource) print type(FrankenStyle) application = service.Application(‘test') root = FrankenStyle() s = server.Site(root) ep = endpoints.serverFromString(reactor, 'tcp:8080') svc = internet.StreamServerEndpointService(ep, s) svc.setName(‘hybridtest') svc.setServiceParent(application) Running siege --benchmark --concurrent=10 --reps=1000 "http://127.0.0.1:8080/“ against the “MyResource” version gets me 5714.29 requests per second across 3 runs. Running siege --benchmark --concurrent=10 --reps=1000 "http://127.0.0.1:8080/“ against the “FrankenStyle” version gets me an average of 5671.61 requests per second across 3 runs. So, not a huge difference, overall, and it might not even be visible over a long enough period of time. Thanks, Daniel

On Tue, Mar 22, 2016 at 10:26 PM, Glyph <glyph@twistedmatrix.com> wrote:

Nothing about 8244 would involve making more hybrid classes. Classes decorated as @oldStyle must be pure old style (the semantics of hybrids are much, much closer to new-style than old-style) so they have to be what they are today. When we flip the switch there will be no more old-style classes at all.

Nothing about #8264's *requirements* leads to hybrid classes. But the implementation makes *every* currently old-style class into a hybrid-class..