lark
lark copied to clipboard
SlottedTree isn't as efficient as it could be
SlottedTree could be a slot-only class, thereby removing any need for an attribute dictionary, which takes up memory. However, by inheriting from Tree, it inherits the attribute dictionary, even if it's not used.
Possible solutions:
- Make the default tree slotted, and have Tree inherit from it. Might improve Lark performance in general
- Use mixins to import the Tree methods into SlottedTree, not inheritance
Hiya @erezsh. I was about to open this issue, glad to see it's already opened. I'm using lark for a long time on a huge project. Recent updates made lark even better and implementing this will be another step in the right direction.
In my opinion, your first suggestion is spot-on as it might improve Lark's performance in general (which is in my experience crucial for so many projects using it). Thoughts?
My main worry is that it will somehow break an existing use of Tree by some of the users of Lark. For example, if they try to assign new properties to a Tree instance.
A "workaround" might be to have a slotted BaseTree, and have Tree inherit from it but unslotted. But then Tree performance stays the same, and there might still be side effects I can't think of.
Well, yes. It will break the API. It just means that this issue should be fixed only in the next API-breaking version.
As for the workaround suggestion, I don't quite see how it improves anything if you still return a Tree instance (and not the slotted one) on the parser's parse method.
I would suggest creating another parsing method inside Lark which will return a slotted tree and adding a DeprecationWarning on the Lark.parse method. When an API breaking version is released, just make the slotted tree the only option.