Reduce heap space used by built in slots.#2445
Conversation
aardvark179
changed the title
gbrail
left a comment
gbrail
left a comment
There was a problem hiding this comment.
Just a few small questions, I see where this is going -- thanks!
| DONTENUM | READONLY, | ||
| BaseFunction::nameGetter, | ||
| BaseFunction::nameSetter); | ||
| Slot<Scriptable> lengthSlot = LENGTH_DESCRIPTOR.createSlot(this, DONTENUM | READONLY); |
There was a problem hiding this comment.
Does it make sense to have the flags (DONTENUM | READONLY etc) in the descriptor rather than in the "createSlot" method itself? I imagine in most cases where we do this we will never change those flags, so you have less code.
There was a problem hiding this comment.
It's a good question. Flags are mutable without replacing a slot in the same way a value can be, which is why I've kept them out of the descriptor half for now. I think it's going to take a few rounds and some more experimentation to work out what the right split is going to end up being.
| } | ||
| } | ||
|
|
||
| setMap(new ImmutableSmallSlotMap<>(lengthSlot, nameSlot, aritySlot, argsSlot)); |
There was a problem hiding this comment.
What are the implications of making this an "immutable" map? Unless I missed something it sounds like it's kind of like making instances of this class and it's subclasses non-extendable, although by throwing a Java rather than a JavaScript exception. JavaScript code loves to muck with the properties of just about everything -- is that really going to work out?
There was a problem hiding this comment.
Making this immutable means that any change to the slots causes the whole slot map to be replaced. This allows for thread safe versions to skip having a lock as they can do a CAS of the map.
| Slot<Scriptable> lengthSlot = LENGTH_DESCRIPTOR.createSlot(this, DONTENUM | READONLY); | ||
| Slot<Scriptable> nameSlot = NAME_DESCRIPTOR.createSlot(this, DONTENUM | READONLY); | ||
| Slot<Scriptable> aritySlot = null; | ||
| Slot<Scriptable> argsSlot = null; |
There was a problem hiding this comment.
Is there a way to do this with some sort of a Builder pattern instead of creating Slot instances directly? I tend to think of Slot as an implementation detail of ScriptableObject although maybe we're past that point already!
There was a problem hiding this comment.
I think a builder is a good idea, but I'm tempted to go one stage further and say we should build a map descriptor and make the map from that. I think it could then be extended to be used by class descriptors.
64d53e3 to
b367743
Compare
|
Okay, created a descriptor API and a builder for that. Although attributes aren't yet part of slot descriptors themselves I have put them into the map descriptor. I'll experiment with how this looks if I try expanding it for our class descriptors as that will force passing values in as well just the owner. |
b367743 to
4f9a593
Compare
2 participants
In production we're seen some cases where we have an extremely high number of function objects. The move away from
IdScriptableObjects causes a significant increase in memory footprint as every function normally has 3 or 4 slots ('length', 'name', 'arity', and 'arguments'). TheBuiltInSlotobjects each had a shallow size of 56, and had to be help in (in our case) aThreadSafeEmbeddedSlotMapwhich added the cost of that object, an array, and a stamped lock to the cost.This PR separates our slot hierarchy into standard slots and compact slots, and introduces small immutable slot maps that can be used for functions. Compact slots have descriptors which can be shared between many slots. We change all the uses of built in slots to use these descriptors, and the creation of the normal properties in
BaseFunctionto create a small immutable slot map.This gets the overhead for function properties down to 160 bytes which is a significant improvement. I believe we can improve this in future work by only generating slot objects when required by our APIs and get the footprint down to at most 32 bytes per function, and extend this immutable slot map concept to the majority of built in prototypes as well.