search

zopefoundation / zope.annotation

Mechanism for storing additional information about objects without modifying their classes
Other
1 stars 8 forks source link

Optimize AttributeAnnotations for speed. #9

Closed jamadden closed 7 years ago

jamadden commented 7 years ago

Fixes #8.

See that issue and the change note for a consequence.

I like that the code is cleaner (DRY) and I like that repeated operations are faster. However, I'm not sure whether or not the consequence (altering views of an object that initially had no annotations, but then an annotation is added to an instance of AttributeAnnotations, but then queried for on a different existing AttributeAnnotations) is worth it.

jamadden commented 7 years ago

I agree about the change potentially being hard to debug in certain cases. The DRY benefits are nice, but not really earth-shaking given the size of the code we're dealing with. Likewise the speed benefits are nice, but don't clearly address a bottleneck. Unless someone can come forward with a case where this makes a real-world difference, I am inclined to withdraw this PR, and be content knowing that the issue was at least looked into (I might make a PR to reference this in a code comment).