+++ This bug was initially created as a clone of Bug #724310 comment 3 +++ I suppose for historical reasons we allow JS_SetReservedSlot and JS_GetReservedSlot to take a non-native object and do nothing in that case. We should replace those checks with asserts. Also JS_SetReservedSlot calls GCPoke presumably to help the shutdown GC to collect more garbage. I suspect that with cycle collector that is not necessary for quite some time and we can drop that.
We do not need those native/non-native asserts. The checks in JSObject::(get|set)ReservedSlot should be sufficient to call the API.
Created attachment 596507 [details] [diff] [review] v1 The patch makes JS_(Get|Set)ReservedSlot a public API around JSObject::(get|set)ReservedSlot. That allowed to remove inlined SetReservedSlot from jsfriendsapi.h. However I have to keep GetReservedSlot as its use is widespread and its removal may have bad performance implications.
Actually, we were using js::SetReservedSlot in the new-binding code. It can get pretty hot during prototype object creation and worse yet object wrapping... Is there actually a reason to remove it?
(In reply to Boris Zbarsky (:bz) from comment #3) > Actually, we were using js::SetReservedSlot in the new-binding code. It can > get pretty hot during prototype object creation and worse yet object > wrapping... > > Is there actually a reason to remove it? The reason is to minimize exposure of JS internals. But if this a hot issue, then surely we can keep it.
Created attachment 596944 [details] [diff] [review] v2 The new patch keeps js::SetReservedSlot and its callers outside js engine as is.
6 years ago