New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cost of validator is prohibitive #28

Closed
cournape opened this Issue Oct 5, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@cournape
Contributor

cournape commented Oct 5, 2015

I noticed that creating instances of attr objects was expensive:

# python >= 3.3, rough timing
import time

from attr import attributes, attr
from attr.validators import instance_of

@attributes
class Arch(object):
    name = attr("name")

@attributes
class ValidatedArch(object):
    name = attr("name", validator=instance_of(str))

def build_platform():
    return Arch("x86")

def timeit(func):
    times = []
    for i in range(10000):
        start = time.perf_counter()
        func()
        times.append(time.perf_counter() - start)
    return "Min cost across 10000 iterations: {:.2f} us".format(min(times) * 1e6)

print(timeit(lambda: Arch("x86")))
print(timeit(lambda: ValidatedArch("x86")))

I see a factor of ~ 50x (0.8us vs 34 us) on my machine. A quick profiling shows that almost all the difference comes from the deepcopy in the attr._make.fields function.

For non trivial classes with a few attributes, the construction cost quickly reaches the hundreds of us.

@hynek

This comment has been minimized.

Member

hynek commented Oct 6, 2015

Hi, thanks for the detailed report!

I indeed haven’t cared much about creation performance until now and it seems like I was a bit too careful by using a public API that uses deep copy there.

Just iterating over __attrs_attrs_ does this:

$ python t.py # before
Min cost across 10000 iterations: 0.55 us
Min cost across 10000 iterations: 31.03 us
$ python t.py # after
Min cost across 10000 iterations: 0.56 us
Min cost across 10000 iterations: 1.31 us

Since the __attrs_attrs__ object is read-only after creation, it should be safe to not lock anything.

Opinions?

@cournape

This comment has been minimized.

Contributor

cournape commented Oct 6, 2015

That certainly seems to fix the most glaring performance issues I noticed with attrs. If __attrs_attrs__ is indeed read-only, that seems like a no-brainer ?

@cournape

This comment has been minimized.

Contributor

cournape commented Oct 7, 2015

This issue is a bit critical for us, is there something more I can do to make this fix appear in a next release ?

@cournape

This comment has been minimized.

Contributor

cournape commented Oct 7, 2015

(did not want to sound too pushy, if it is difficult to make it happen soon, we can fairly easily backport this in a bundled attr until a new official release)

@hynek

This comment has been minimized.

Member

hynek commented Oct 7, 2015

If you’d have pinged the PR yesterday (GH doesn’t notify me of new pushes…), I might have made a quick release today but now I’m packing up for PyCon JP and won’t be back until next Wednesday. I will try to find a minute to push it out tho.

@cournape

This comment has been minimized.

Contributor

cournape commented Oct 7, 2015

Have fun at PyCon JP. Looks like this year's location is near tsukuji market, perfect for great sushi before the conference

@hynek

This comment has been minimized.

Member

hynek commented Oct 19, 2015

Closed by #29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment