isPlainObject optimize

[lioeaet]

hi

optimization of isPlainObject function is in this pr

sorry for such few words, language barrier it is

performance tests:

const vars = {
  'date': new Array(7777777).fill(new Date()),
  'array': new Array(7777777).fill([1, 2, 3]),
  'plain obj': new Array(7777777).fill({ x: 1, y: 2 }),
  'model': new Array(7777777).fill(new (class Book {})),
  'null': new Array(7777777).fill(null),
  'undefined': new Array(7777777).fill(undefined),
  'string': new Array(7777777).fill(''),
  'true': new Array(7777777).fill(true),
  'false': new Array(7777777).fill(false),
}

function measure(func, arr) {
  const start = performance.now()
  for (let x of arr) func(x)
  return performance.now() - start
}

function x(obj) {
  if (typeof obj !== 'object' || obj === null) return false

  let proto = obj
  while (Object.getPrototypeOf(proto) !== null) {
    proto = Object.getPrototypeOf(proto)
  }

  return Object.getPrototypeOf(obj) === proto
}

function y(obj) {
  if (typeof obj !== 'object' || obj === null) return false

  const proto = Object.getPrototypeOf(obj)
  return Boolean(proto && !Object.getPrototypeOf(proto))
}

const result = {}
for (let k in vars) {
  result[k] = {
    x: measure(x, vars[k]),
    y: measure(y, vars[k]),
  }
}
console.log(result)

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e7c1c7e:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration

[phryneas]

This will fail with Object.create(null), which is a perfectly fine plain object.
See #2599 for some edge case discussions.

[lioeaet]

great comment, thank you!

there is a complex logic with loop, and make things simpler is profit itself

think we should do this if can iterate less without troubles

case with no proto is fixed in new commit, also added tests for it

speed results almost same

const vars = {
  'date': new Array(7777777).fill(new Date()),
  'array': new Array(7777777).fill([1, 2, 3]),
  'plain obj': new Array(7777777).fill({ x: 1, y: 2 }),
  'model': new Array(7777777).fill(new (class Book {})),
  'null': new Array(7777777).fill(null),
  'undefined': new Array(7777777).fill(undefined),
  'string': new Array(7777777).fill(''),
  'true': new Array(7777777).fill(true),
  'false': new Array(7777777).fill(false),
}

function measure(func, arr) {
  const start = performance.now()
  for (let x of arr) func(x)
  return performance.now() - start
}

function x(obj) {
  if (typeof obj !== 'object' || obj === null) return false

  let proto = obj
  while (Object.getPrototypeOf(proto) !== null) {
    proto = Object.getPrototypeOf(proto)
  }

  return Object.getPrototypeOf(obj) === proto
}

function y(obj) {
  if (!obj) return false

  const proto = obj.__proto__
  return !proto || !Object.getPrototypeOf(proto)
}

const result = {}
for (let k in vars) {
  result[k] = {
    x: measure(x, vars[k]),
    y: measure(y, vars[k]),
  }
}
console.log(result)

[phryneas]

I'm just running this in Firefox, and your solution is significantly slower in many cases and only noticeably faster in the new Date case.

There is a way to speed up the existing logic by skipping one Object.getPrototype call with

  if (typeof obj !== 'object' || obj === null) return false

  let proto = obj,
      next
  while ((next = Object.getPrototypeOf(proto)) !== null) {
    proto = next
  }

  return Object.getPrototypeOf(obj) === proto

which seems to be a much safer change that is not potentially breaking.

---

Please let's step back for a while: Why are you optimizing this? Does this make anything any slower in any real life application you have?

Any change we introduce here might break edge cases we don't know about. Without having a really good reason to, I don't think that code like this should ever be touched.

And a microoptimization is rarely a good reason - so I'm asking again: do you see real impact on this in a real-life application? What is a use case where this makes a difference?

[phryneas]

Please stop adding more optimizations.

We will not accept this pull request unless you give us an incredibly good reason to introduce potentially-breaking code to millions of installations.

I have asked multiple times why you are doing this PR in the first place.

Please start talking.

[lioeaet]

You right, in firefox i can see slower results too.

Can speed up it with replacing first line of code. Results are only positive.

There is no question of UX improvements. Surplus iterations spend energy in real world. It fraught with some surplus damage to ecology. Redux has 7.5 millions of weekly downloads. It is not catastrophic scope for little loop, but it is and can be not.

You shown an hidden error in my code, thank you. But javascript is not endless. Function is simple. We can fix it instantly in case of error.

With excluding cycle code will pretty easer, people will not spend their time on puzzling it.

Totally this pr:

• prevents unnecessary damage to nature
• saves code learners time
• crop code a little
• may not to include potential bugs, may include (and be fixed)

[phryneas]

Generally: how many actions are you dispatching that this results in any relevant performance improvements? This check should already work millions per second without problems.

[markerikson]

Hi. We appreciate the PR, but the current implementation is okay as-is. This function is not going to be a perf bottleneck.