Closed Jordan-Cottle closed 3 years ago
This looks really exciting!... I'd read about slots probably for 2-3 hours but the confidently using them part I hadn't got to. I didn't want to put them in blindly without fully understanding the repurcussions. I kinda sussed where they might go in the file and was getting close to your solution. but hadn't took the next step yet. I'll try to pull this down and run some of my examples through it etc. I'm sure it's fine and your explanation is really helpful in me understanding them.
I'm glad I was able to help!
Using __slots__
essentially is just a trade off of some of the dynamic power of python objects (which doesn't appear to be in use here, and should really only be used where there's a good reason to do so) for better performance.
By default, objects can have whatever attributes assigned to them. Which is why you can just plop self.<whatever> = <some value>
anywhere in a method (like __init__
or any other function you define in a class without the @classmethod
or @staticmethod
). Whenever an attribute is created or accessed, it goes to the object's __dict__
. So foo.bar
would look up foo.__dict__["bar"]
if foo
was an object with attribute bar
.
When you use __slots__
the class essentially automatically creates an @property
with a setter for every name in the list you provide and prevents creating a __dict__
from being created (unless you add "__dict__"
to the __slots__
list) so that no unexpected attributes can be defined on the fly like they could before.
Since dictionary objects are data structures that use up more space than just the data they hold, and accessing things through them has some overhead (but is generally an O(1) operation), using slots can save memory by not allocating memory for __dict__
and increase attribute lookup performance by skipping __dict__[attribute_name]
lookups.
It's generally only considered/done for classes that have a ton of instances created or lots of attribute accesses that need to be fast.
that's a really helpful explanation and is really good and helping me get it. I guess my question with them was. if someone extended a tag. would their extended thing be able to have editable properties. as that would make it a fine i felt as most cases could be covered that way if a person really needed to extend a tag. but really they should just be returning them.
i'd like to also try and benchmark it on some of the bgger templates i have to see what the performance gains are. I didn manage to parse a page once into 5k nodes and was getting memory issues. so I had wondered if investigating this would help there. But really what i shoudl do is try set up some tests to show peformance gains. then consider where else i may be able to add them.
Benchmarking things before attempting to optimize them is a pretty good idea. It's also nice to know how much of an impact a performance change makes.
For inheritance, the __slots__
vs __dict__
question doesn't really matter. Since setting __slots__
on tag
only affects the names in the tag
class. Any subclasses should be able to make independent decisions about any attribute that they add as well.
Here's a quick gist you can see that shows that subclasses that don't mention __slots__
will behave as expected.
amazing.! that's exactly the test and question I wanted to try. It looks like this should be good. I'll be adding this in at some point soon but will probably play a little first with the benchmarking thing as I've wanted to do a bit of that. thanks so much by the way. I'm feeling a lot better about slots
also thanks again for the timeout methods. they are huge wins. we had a huge number of clones yesterday so it must have been highly anticipated.
You're welcome!
I'm glad I was able to help.
@Jordan-Cottle also a bit off topic. but your decorators for events idea is really good. as you can see on this projects I was using eventlisteners. https://github.com/byteface/vnc2browser/blob/master/vnc2browser.py and also considered not needing all of domonic just for listeners. but I had not had the decorators idea. nice work.
Thanks!
Something you could also do to avoid the "all of Domonic just for X" problem in general would be split up the Dominic packages into their own projects.
Or publish them separately from the same GitHub project (I've not set up something like this before).
That way you can grab the pieces separately if you want.
I think there's also a way to set up a package with optional extras. So you could theoretically set up Domonic to allow selecting it's components (and limit which of it's third party packages are needed depending on what components are needed).
I've not got a lot of experience with publishing packages, so I don't know exactly how to do any of the options beyond just splitting up the package. Just making same observations from how I've seen some of the more complex/larger packages I've used have been set up.
I'll probably have to figure it out for that events package though. The primary goal on my roadmap there is to enable the event publishers and subscribers to be on totally separate machines and that will likely need to involve some kind of server to handle routing event messages. Wouldn't want all the server stuff to be required by every install if most places only need basic http stuff to send/receive messages.
ye someone else had suggested me branching the terminal stuff.
I just had a go at forking off javascript to a new package called esx https://github.com/byteface/esx
think ill try seperate out a few concerns. as also helps simplify.
It looks like issue #21 mentions that you wanted to use
__slots__
in this project. I'm not familiar with the structure of the project or where all the classes are defined, so this PR only adds__slots__
for the one class that had the todo comment for it.All of the rest of the classes should be just as simple. You just have to list out all of the expected instance attributes for each class. If a class is simple and just assigns all of it's attributes normally with
self.<name> = ...
in it's__init__
or elsewhere, then you can just search forself.\w+ =
to locate names easily.Using
__slots__
instead of the default__dict__
for attributes is pretty straightforward unless you have classes where instance attributes are dynamically set (not just appear to be set with a__getattr__
or__getattribute__
that intercepts attribute access).I'm not sure this change warrants any extra tests since the existing test suite covers all of the attribute assignments in the
tag
class