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

perf: preinitialize typeCheck RegExp #10990

Open
wants to merge 1 commit into
base: dev
from

Conversation

@SebastianSpeitel
Copy link

SebastianSpeitel commented Jan 7, 2020

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe: Performance improvement

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

After looking at the performance of one of my apps, I noticed, the function, that took the most time summed up was getType() , defined in [1] taking 200ms. I noticed the regular expression getting created for each call and measured again including the proposed improvement, which took the time down to 150ms. To make sure I changed it back to confirm and it again timed at 200ms.

[1]

vue/src/core/util/props.js

Lines 182 to 185 in b07087d

function getType (fn) {
const match = fn && fn.toString().match(/^\s*function (\w+)/)
return match ? match[1] : ''
}

@SebastianSpeitel SebastianSpeitel changed the title Predefine typeCheck RegExp perf: preconstruct typeCheck RegExp Jan 7, 2020
@SebastianSpeitel SebastianSpeitel changed the title perf: preconstruct typeCheck RegExp perf: preinitialize typeCheck RegExp Jan 7, 2020
@SebastianSpeitel

This comment has been minimized.

Copy link
Author

SebastianSpeitel commented Jan 7, 2020

Maybe further improvements could be made by removing the regular expression completely and using standard string manipulation methods.

@posva
posva approved these changes Jan 8, 2020
@SebastianSpeitel

This comment has been minimized.

Copy link
Author

SebastianSpeitel commented Jan 14, 2020

Is there anything else, that needs to be done before merging? @posva

@posva

This comment has been minimized.

Copy link
Member

posva commented Jan 14, 2020

@SebastianSpeitel You don't need to do anything else, thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.