search

domenic / get-originals

A web platform API that allows access to the "original" versions of the global built-in objects' properties and methods
28 stars 1 forks source link

Discussion of efficient implementation seems to be making assumptions that are not obvious #11

Closed bzbarsky closed 5 years ago

bzbarsky commented 6 years ago

1) It's not clear why having a separate API for callOriginalStaticMethod() allows more efficient implementation, necessarily. It seems like it needs to do basically the work that callOriginalMethod does, plus the work of looking up the actual object.

2) The "constructors" subset may not actually match any category that exists in implementations, so restricting to it may in fact be slower than something that does map onto implementation concepts.

3) The inability to get and cache original functions means that while it may be possible to avoid creating the function in the first place calling it multiple times requires a lot more time than just getting it once and then calling the resulting actual Function multiple times.

I don't quite know what assumptions are being made here about access patterns and the way things work in implementations, and it's not clear to me that they're the right assumptions.

domenic commented 6 years ago

I think the biggest takeaway here is that I didn't think through the value of caching original functions. When we swung toward the instance-based version, away from my first getOriginal("Node.prototype.getRootNode")-style design, I assumed we would just re-invoke callOriginalMethod() etc. every time. But as you point out, that's not an obvious tradeoff.

I'm not sure what kind of caching-friendly design would also work with an instance-based one, but it's worth thinking further on.

I definitely posted this as a first draft based on the naive assumptions that I've gathered by being only implementation-adjacent, and not deep in the implementation. Thanks so much for your help so far in refining them; I'll continue to iterate based on your feedback and that of the V8/Blink bindings folks.

bzbarsky commented 6 years ago

Of course.

Note that the SpiderMonkey folks actually kind of liked the "don't expose any of the function objects except the constructors" aspect, because they don't want to deal with caching those objects. I think they would be ok with returning a newly minted object each time and letting user code cache it, though.... Though that might have its own confusion issues.