Currently `Object.create(null);` is 97% slower than `var x = {};` We want to use Object.create(null) more often for sanity/security reasons, but the performance kick is making it a trade off. It would be awesome if we could get those two comparable.

Ops/sec on current nightly: Object.create(null) - 3,312,898 {} - 120,804,454

(Changing link to newer version of the benchmark under the assumption that you meant to link to that. The numbers you give match my results, and the linked version reduces the `var x = {}` case to an empty-loop performance test.)

|var x = {}| does an MNewObject, which does all the work in jitcode (with an out of line stub, as far as I can tell). Object.create does a CallNative into the VM to do the work. That pretty much explains the difference...

(In reply to Zibi Braniecki [:gandalf] from comment #0) > We want to use Object.create(null) more often for sanity/security reasons, > but the performance kick is making it a trade off. Apart from the speed issue, how would this be more secure?

(In reply to Nicolas B. Pierron [:nbp] from comment #4) > Apart from the speed issue, how would this be more secure? Using Object.create(null), you can create objects which you can pass to code you don't necessarily trust without automatically leaking Object.prototype and hence the world.

In our case, we have a key, that is a string, but is coming from a potentially unsecure code (from language package), and we do things like: if (key in obj) { return obj[key]; } Now, if we can have obj be null-prototyped, that saves us the need to do hasOwnProperty on each and every operation on the obj.

Can you take a look at this or anybody else of course? It would be awesome if we could inline Object.create and have it be as fast as object literals.

I'm seeing the hit in Lo-Dash too. We use an object created by Object.create(null) has a hash to complement our Set use. The object created by Object.create(null) is used for non-string-non-object values and Set is used for the rest. See http://jsperf.com/array-object-unique/3#chart=bar

I believe the Lo-Dash perf issue isn't with the cost of `Object.create(null)` but the cost of accessing properties on the object created from it.

(In reply to Tom Schuster [:evilpie] from comment #7) > Can you take a look at this or anybody else of course? It would be awesome > if we could inline Object.create and have it be as fast as object literals. I'll try to get to it this week. (In reply to John-David Dalton from comment #9) > I believe the Lo-Dash perf issue isn't with the cost of > `Object.create(null)` but the cost of accessing properties on the object > created from it. Yes, that's possible; objects created with Object.create(null) all share the same TypeObject atm, I'll fix that too.

This patch gives Object.create(null) an allocation-site specific TypeObject. It also inlines Object.create(x), if TI knows x matches the template object's proto (for Object.create(null) that should always work). I didn't try http://jsperf.com/object-create-null-vs-literal/3 yet but according to some shell micro-benchmarks, Object.create(null) should now be about as fast as (or slightly faster than) {} I also removed the unused NewInitObjectWithClassPrototype function.

On http://jsperf.com/object-create-null-vs-literal/3 I see numbers like this without the patch: create(null) : 3,100,404 {} : 187,740,456 create(O.prototype): 3,427,942 create({}) : 711,183 with the patch, I get something like: create(null) : 209,094,228 {} : 182,986,755 create(O.prototype): 4,091,368 create({}) : 700,512 so yes, looks good.

Thanks for testing this! (In reply to Boris Zbarsky [:bz] from comment #12) > create(O.prototype): 4,091,368 create() got a lot faster here but the main problem is the property set: it requires dynamic slots and our SetProperty IC doesn't support this yet (bug 935932, we should really fix that). For create(null) the patch uses GuessObjectGCKind (we also use this for {}), which gives you an object with some fixed slots, so that we don't need dynamic slots if we add a small number of properties. We should probably also do that for Object.create(non-null)... > create({}) : 700,512 This is expected to be slow(er) because (1) allocates two objects and (2) Ion doesn't inline this case (proto will be different each time + {} will be in the nursery).

Yeah, I wasn't expecting create({}) to be anything sane; it seems like a really bizarre thing to do anyway.

With this patch we also use GuessObjectGCKind(0) in the create(non-null) case. I think it's reasonable for Object.create to match {}.

https://hg.mozilla.org/integration/mozilla-inbound/rev/13f7779c31ed Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a3f8402324d

Comment on attachment 8553721 [details] [diff] [review] Patch Review of attachment 8553721 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/builtin/Object.h @@ +23,5 @@ > bool > obj_valueOf(JSContext *cx, unsigned argc, JS::Value *vp); > > +PlainObject * > +ObjectCreateImpl(JSContext *cx, HandleObject proto, NewObjectKind newKind = GenericObject, Missing #include "jsobj.h" ... JS::HandleObject proto ... @@ +24,5 @@ > obj_valueOf(JSContext *cx, unsigned argc, JS::Value *vp); > > +PlainObject * > +ObjectCreateImpl(JSContext *cx, HandleObject proto, NewObjectKind newKind = GenericObject, > + HandleTypeObject type = js::NullPtr()); Missing #include "gc/Rooting.h"

Comment on attachment 8553721 [details] [diff] [review] Patch Review of attachment 8553721 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/Recover.h @@ +599,5 @@ > > class RNewObject MOZ_FINAL : public RInstruction > { > private: > + MNewObject::Mode mode_; Missing #include "jit/MIR.h"

(In reply to Yuan Pengfei from comment #19) > Comment on attachment 8553721 [details] [diff] [review] > Patch > > Review of attachment 8553721 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/jit/Recover.h > @@ +599,5 @@ > > > > class RNewObject MOZ_FINAL : public RInstruction > > { > > private: > > + MNewObject::Mode mode_; > > Missing #include "jit/MIR.h" Which is exactly why we should not use MIR Instruction references in Recover.h, as this file in included by vm/Stack.h and we should avoid such dependency. As we have an assertion in Recover.cpp, I'll suggest using uint8_t instead.

I just tested this in the browser and noticed Object.create(Object.prototype) is still slow. Turns out I forgot to pass allocKind to NewObjectWithGivenProto :( This patch fixes that and improves the test below from 300 ms to 9 ms. function f() { var t = new Date; for (var i=0; i<1000000; i++) { o = Object.create(Object.prototype); o.foo = i; } print(new Date - t); } f();

Part 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/18d13560baba