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: improve the performance of timezone #2542

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

tmkx
Copy link

@tmkx tmkx commented Jan 3, 2024

it's 20% faster in a 10,000-invocation loop test

❯ node ./node.test.mjs
✔ dev (166.546375ms)
✔ perf (133.484125ms)

import assert from 'node:assert'
import test from 'node:test'

import dayjs from './dayjs.min.js'
import utc from './plugin/utc.js'
import timezone from './plugin/timezone.js'

import dayjsPerf from './perf/dayjs.min.js'
import utcPerf from './perf/plugin/utc.js'
import timezonePerf from './perf/plugin/timezone.js'

dayjs.extend(utc)
dayjs.extend(timezone)

dayjsPerf.extend(utcPerf)
dayjsPerf.extend(timezonePerf)

test('dev', () => {
  for (let i = 0; i < 10000; i++) {
    const offset = dayjs
      .tz('2024-01-03 00:00:00', 'America/New_York')
      .utcOffset()
    assert.equal(offset, -300)
  }
})

test('perf', () => {
  for (let i = 0; i < 10000; i++) {
    const offset = dayjsPerf
      .tz('2024-01-03 00:00:00', 'America/New_York')
      .utcOffset()
    assert.equal(offset, -300)
  }
})

@tmkx
Copy link
Author

tmkx commented Jan 10, 2024

It can significantly improve the first paint time in a heavy-dependent app like Calendar.

@tmkx
Copy link
Author

tmkx commented Jan 30, 2024

any feedback?

@buza-me
Copy link

buza-me commented Feb 7, 2024

I'm not the library author, but: Your test is made in one script. You need to run separate scripts for benchmarks. 10k iterations on 'dev' version gives everything a decent warm-up. I'm saying "everything", because the difference may even come from node:assert optimized by v8. I believe that removing string concatenation improves the script speed, but still your benchmark is not reliable imo.
Second thing is: How can someone run your test snippet, if they do not have the files?

import dayjs from './dayjs.min.js'
import utc from './plugin/utc.js'
import timezone from './plugin/timezone.js'

import dayjsPerf from './perf/dayjs.min.js'
import utcPerf from './perf/plugin/utc.js'
import timezonePerf from './perf/plugin/timezone.js'

You should probably link some repo that contains those files.

@tmkx
Copy link
Author

tmkx commented Feb 7, 2024

I agree with you that the number is not very precise, it's affected by many factors, including warm-up, execution order, cpu occupation, etc. I showed this just for simplicity and sorry for the confusion.

I've set up a benchmark using Vitest, you can run npm run bench with https://github.com/tmkx/dayjs/tree/perf/timezone-bench, and here is my result:

 ✓ bench/timezone.bench.ts (2) 1236ms
     name             hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · perf      40,160.84  0.0230  0.5659  0.0249  0.0245  0.0385  0.0592  0.1507  ±0.45%    20081   fastest
   · baseline  33,307.05  0.0276  1.5496  0.0300  0.0292  0.0548  0.0710  0.1799  ±0.75%    16654  
import { bench } from 'vitest'

bench('perf', () => {
  dayjsPerf.tz('2024-01-03 00:00:00', 'America/New_York').utcOffset()
})

bench('baseline', () => {
  dayjs.tz('2024-01-03 00:00:00', 'America/New_York').utcOffset()
})

the baseline and perf files are the output of npm run build in two branches.

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

2 participants