feat: make ABIType a singleton type#245
feat: make ABIType a singleton type#245BobTheBuidler wants to merge 12 commits intoethereum:mainfrom
Conversation
Since ABIType objects are immutable, and downstream libraries rapidly create, use, and destroy them, I've made ABIType into a singleton class by giving it a specialized metaclass. This allows caching stuff like my previous PR to work even better, and will not cause any leaky memory issues because the number of types is constrained.
|
I liked this PR a lot more before I found out about the hash issue... I'm not sure if it will actually be an improvement in its current form, some testing is needed. I might need to table this for now and make a PR in parsimonius repo to add a hash method to Node objects If nodes become hashable, this would again become a clear value-add |
|
I have a PR here to add a hash method, we'll see In the meantime, I do think this might still be a good addition. The only test that was substantially slower was test_is_encodable_returns_true_for_random_valid_tuple_values which does make sense, considering this PR was only intended to give performance benefits on the 2nd and subsequent accesses of a particular type and "random valid tuple values" is pretty much the polar opposite of that A few other tests were marginally slower, which makes sense for the same reason. I do not consider either slowdown to be problematic. A good benchmark to determine the effectiveness of this PR would be to repeatedly encode and decode a few commonly used evm types in a hot loop to simulate real world usage patterns. There isn't much point to benchmarking "encodable_returns_true_for_random_valid_tuple_values"! lol |
Since ABIType objects are immutable, and downstream libraries rapidly create, use, and destroy them, I've made ABIType into a singleton class by giving it a specialized metaclass.
This allows caching stuff like my previous PR to work even better, and will not cause any leaky memory issues because the number of types is constrained.
What was wrong?
Related to Issue #
Closes #
How was it fixed?
Todo:
Cute Animal Picture