-
-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve performance #16
Conversation
index.js
Outdated
return false; | ||
} | ||
|
||
const prototype = Object.getPrototypeOf(value); | ||
return prototype === null || prototype === Object.prototype; | ||
return (prototype === null || prototype === Object.prototype) && !(Symbol.toStringTag in value) && !(Symbol.iterator in value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #12 (comment)
benchmark.js
Outdated
new Proxy({}, {}) | ||
]; | ||
|
||
const runLoop = function (value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const runLoop = function (value) { | |
const runLoop = value => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed 👍
benchmark.js
Outdated
import {inspect} from 'node:util'; | ||
import isPlainObject from 'is-plain-obj'; | ||
|
||
const runBenchmarks = function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const runBenchmarks = function () { | |
const runBenchmarks = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed 👍
Can you fix the conflict? |
Fixed. I have also added a small additional performance improvement at 58de3a2. Before:
After:
Note: you can see the time for objects that are not plain objects is now roughly twice slower compared to the first benchmarks published above. This is unrelated to this PR: this is due to the cross-realm PR (#14) which added an additional |
For completeness, those are benchmarks before/after both this PR and the cross-realm PR (#14). Before:
After:
|
Nice work :) |
Would you be interested in applying your changes to https://github.com/sindresorhus/is/blob/dc99f7cd4a8ad309d4c945b21816132dbc09ef92/source/index.ts#L287-L297 too? |
Sure! Done at sindresorhus/is#169 |
Fixes #12
This improves the performance.
Benchmarks on Node 18.3.0 on my machine, before the change:
After the change:
In a nutshell:
symbol
,bigint
) are 60 times fasterundefined
,null
,number
,string
,boolean
) and functions are 5 times fasterSet
,Promise
,ArrayBuffer
,Math
,Intl.*
,arguments
) are 2-4 times fasterProxy
is 30% fasterObject.create(null)
is 10% slowerArray
,RegExp
,Error
,Date
are 30% slower. This is the main drawback. However, this seems worthwhile based on the other improvements.new Object()
is twice slower. It is rarely used though.