Skip to content
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

hash function does multiple toStrings when it could do 1 #4090

Open
garyhuntddn opened this issue Feb 9, 2025 · 1 comment
Open

hash function does multiple toStrings when it could do 1 #4090

garyhuntddn opened this issue Feb 9, 2025 · 1 comment

Comments

@garyhuntddn
Copy link

Bug report

Description / Observed Behavior

I was looking through the codebase and specifically src/_internal/utils/hash.ts

There are 3 calls to isObjectType which in turn executes OBJECT.prototype.toString.call(value).

Expected Behavior

Given that OBJECT.prototype.toString.call(value) will be constant across these three calls the code could be changed to call that toString just once.

Repro Steps / Code Example

const isObjectType = ( value: any, type: string ) => {
  return getTypeName( value ) === `[object ${ type }]`
}

...
const isDate = isObjectType( arg, 'Date' );
const isRegex = isObjectType( arg, 'RegExp' );
const isPlainObject = isObjectType( arg, 'Object' );
...

becomes

const getTypeName = ( value: any ) => {
  return Object.prototype.toString.call( value )
}

const isObjectTypeName = ( typeName: string, type: string ) => {
  return typeName === `[object ${ type }]`
}

...
const typeName = getTypeName( arg );
const isDate = isObjectTypeName( typeName, 'Date' );
const isRegex = isObjectTypeName( typeName, 'RegExp' );
const isPlainObject = isObjectTypeName( typeName, 'Object' );
...

Additional Context

This is a micro performance optimisation which I don't generally condone - but in this case - every use of stableHash goes through those three lines before performing any other work. Therefore it's a hot spot in the code and I believe worthy of examination.

Of the work done on every call - these three lines are the expensive ones.

In my brief experimentation the performance gain is considerable when there is a cache hit (nearly 3x faster), and marginal when there is a miss (the other work performed on a miss includes calls to .sort and .toJson which will be very computationally expensive).

Happy do turn this into a pull request if you're in agreement with the change.

@shuding
Copy link
Member

shuding commented Feb 11, 2025

Thanks, feel free to open a PR for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants