testing-cabal / testtools

Testtools - tasteful testing for python
https://testtools.readthedocs.io/en/latest/
Other
95 stars 88 forks source link

Revert usage of pyrsistent and _TestRecord #220

Closed therve closed 8 years ago

therve commented 8 years ago

The _TestRecord class is pretty big performance hit, as it adds a fair amount of indirection for each test execution. This reverts its creation and remove the dependency on pyrsistent.

Close #219

Review on Reviewable

rbtcollins commented 8 years ago

I'll apply this in a few days so that jml and thomir get a chance to review for other ways to address it, but yeah, looks straight forward enough to me; sad tho.

jml commented 8 years ago

I'd have done this by changing the pyrsistent class to a regular one and keeping the rest: I think it's a lot better than an implicit dict. However, I don't think that strongly enough to object, so +0 from me.

rbtcollins commented 8 years ago

@therve could you perhaps test the performance with a regular class?

therve commented 8 years ago

I tried with https://github.com/testing-cabal/testtools/pull/221.

It's still a bit slower, but more in the 10% range rather than the 200% range.