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(core): filter rulesDynamic in resolveConfig #1534

Merged
merged 1 commit into from Sep 4, 2022
Merged

Conversation

QiroNT
Copy link
Member

@QiroNT QiroNT commented Sep 3, 2022

Changes to @unocss/core

  • rulesDynamic is being filtered and reversed in resolveConfig, this yields a ~10ms improvement on the new benchmark.
  • extractorSplit now first de-dupe via set then test the string. The impact is minimal, but faster on dedicated benchmark.
  • Extractor results are merged via a for-loop instead of a forEach. The impact is again minimal, but it's nice to have.

Changes to the benchmark

  • The new benchmark measures the full build time, from buildStart to closeBundle, instead of buildEnd.
  • The new benchmark runs for 200 times instead of 50 times to give a more consistent result.
  • Metric can also be set to percentiles, and it's now set to show the 75th percentile result which is more representative of the real-world performance.
  • The source now includes a bit of HTML to test the splitter. Tho I think it should also have some garbage, I'll leave that to a future person.

The old benchmark is... flawed at best.
Since both TailwindCSS and WindiCSS generate their CSS before buildEnd, and UnoCSS generates it's CSS in generateBundle which is after buildEnd. The benchmark doesn't actually measure the full build time for UnoCSS, but only part of the extraction time.
To make the benchmark result more realistic, and better for future comparisons, the new one measures the full build time, which now includes CSS generation time.

@QiroNT QiroNT requested a review from antfu as a code owner September 3, 2022 15:31
@netlify
Copy link

netlify bot commented Sep 3, 2022

Deploy Preview for unocss failed.

Name Link
🔨 Latest commit 67296ed
🔍 Latest deploy log https://app.netlify.com/sites/unocss/deploys/6313734da96b05000808f772

Copy link
Member

@zyyv zyyv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggest

@@ -1,12 +1,12 @@
import type { Extractor } from '../types'
import { isValidSelector } from '../utils'

export const splitCode = (code: string) => code.split(/\\?[\s'"`;={}]+/g).filter(isValidSelector)
export const splitCode = (code: string) => [...new Set(code.split(/\\?[\s'"`;={}]+/g))].filter(isValidSelector)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it does the same job as his name, just to simply split the code. Deduplication should be handed over to the next step. (Just my opinion 😄)

@antfu
Copy link
Member

antfu commented Sep 4, 2022

Nice catch! Thanks

@antfu antfu merged commit fa0bf07 into main Sep 4, 2022
@antfu antfu deleted the perf/opt-rules-dynamic branch September 4, 2022 06:46
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

Successfully merging this pull request may close these issues.

None yet

3 participants