jkbrzt / rrule

JavaScript library for working with recurrence rules for calendar dates as defined in the iCalendar RFC and more.
https://jkbrzt.github.io/rrule
Other
3.24k stars 506 forks source link

`between` is 10X slower than before when there is a TZID in the rrule. #580

Open FHachez opened 1 year ago

FHachez commented 1 year ago

between is 10X slower than before when there is a TZID in the rrule.

Between the version 2.6.4 and the version 2.7.0+, between is 10X slower if there is a TZID in the rrule.

With the following benchmark, I observe the following performance:

Even more striking, without the TZID, it only takes 0.7ms on average with the version 2.7.4.

import { performance } from 'perf_hooks';
import { rrulestr } from 'rrule';

function rruleGenerateDates() {
  // Same speed for both rrule 2.6.4 and 2.7.0+
  //const recurence = `DTSTART;TZ=Europe/Brussels:2022-04-04T10:00;RRULE;FREQ=WEEKLY;BYDAY=WE;INTERVAL=1`;

  // rrule 2.6.4 - 4ms average on Mac M1
  // rrule 2.7.0+ - 50ms average on Mac M1
  const recurence = `DTSTART;TZID=Europe/Brussels:20210324T110000\nRRULE:INTERVAL=1;FREQ=WEEKLY;BYDAY=WE,TH`;
  const rule = rrulestr(recurence);
  let dates = rule.between(new Date('2022-04-04T10:00'), new Date('2023-04-04T11:00'));
}

// Set the number of runs.
const runs = 100;

// Function to benchmark a function.
async function benchmark(func: () => any, runs: number) {
  let start = performance.now();

  for (let i = 0; i < runs; i++) {
    await func();
  }

  let end = performance.now();

  return (end - start) / runs;
}

// Run the benchmark on both versions.
async function runBenchmark() {
  let duration = await benchmark(rruleGenerateDates, runs);

  console.log(`Old version average time: ${duration} ms`);
}

runBenchmark();
dorfman commented 10 months ago

Looks like the Luxon dependency was removed in PR #508 to reduce load times in-browser. Luxon is really good at handling timezones, especially when compared to the native Date library. So that’s a bummer.

W0lfEagle commented 10 months ago

Also noticed huge performance issues. We're seeing vast improvements by using the rrule-rust package.

davidgoli commented 8 months ago

Yeah, there isn't a great native replacement for Luxon's TZ functionality. Most of the cost is in the dateInTimeZone functionality:


const dateTZtoISO8601 = function (date: Date, timeZone: string) {
  // date format for sv-SE is almost ISO8601
  const dateStr = date.toLocaleString('sv-SE', { timeZone })
  // '2023-02-07 10:41:36'
  return dateStr.replace(' ', 'T') + 'Z'
}

export const dateInTimeZone = function (date: Date, timeZone: string) {
  const localTimeZone = Intl.DateTimeFormat().resolvedOptions().timeZone

  // Date constructor can only reliably parse dates in ISO8601 format
  const dateInLocalTZ = new Date(dateTZtoISO8601(date, localTimeZone))
  const dateInTargetTZ = new Date(dateTZtoISO8601(date, timeZone ?? 'UTC'))
  const tzOffset = dateInTargetTZ.getTime() - dateInLocalTZ.getTime()

  return new Date(date.getTime() - tzOffset)
}

I see a 25% speed up by moving this line to the top-level scope:

const localTimeZone = Intl.DateTimeFormat().resolvedOptions().timeZone

So that's worth doing. Will have to investigate more to see what else can be sped up.

newOlli commented 7 months ago

Hello!

I'm having the same problem. After doing some research, I discovered the following:

Rule1 : freq=DAILY;dtstart;TZID=Europe/Helsinki:20200102T020000;interval=1;byweekday=MO,TU,WE,TH,FR,SA,SU; Rule2 : freq=DAILY;tzid=Europe/Helsinki;interval=1;byweekday=MO,TU,WE,TH,FR,SA,SU;byhour=2;byminute=0;bysecond=0;

Although both rules provide the same dates, Rule 1 takes 681 ms locally and Rule 2 takes 0.79ms.

I then discovered that the between call takes longer if the rules dtstart and betweens parameter after are far apart. For instance, the call is relatively quick if I have a rule with 1.1.2020 as the start date and I call between 1.1.2021 and 1.2.2021 as the after and before dates. However, using 1.1.2030 and 1.2.2030 as betweens parameters causes the function to execute quite slowly. This is only noticeable if TZID is defined in the rule. The call still takes longer if TZID is not defined, but the difference is not as bad.

I am using rule 2.8.1 with the test code below but the same problem is also in 2.7.2.

import { performance } from "perf_hooks";
import { rrulestr } from "rrule";

function rruleGenerateDates(rulesting: string, customAfterDate?: Date) {
  const rule = rrulestr(rulesting);

  if (customAfterDate) {
    const before = new Date();
    before.setDate(customAfterDate.getDate() + 1 * 7);
    let dates = rule.between(customAfterDate, before);
  } else {
    let dates = rule.between(new Date("2022-04-04T10:00"), new Date("2023-04-04T11:00"));
  }
}

// Set the number of runs.
const runs = 100;

// Function to benchmark a function.
async function benchmark(func: (rule: string, customAfterDate?: Date) => any, rule: string, runs: number, customAfterDate?: Date) {
  let start = performance.now();

  for (let i = 0; i < runs; i++) {
    await func(rule, customAfterDate);
  }

  let end = performance.now();

  return (end - start) / runs;
}

async function runBenchmark(runs: number) {
  cconst ruleWithStartTime = `freq=DAILY\ndtstart;TZID=Europe/Helsinki:20200102T020000\ninterval=1\nbyweekday=MO,TU,WE,TH,FR,SA,SU`;
  let duration = await benchmark(rruleGenerateDates, ruleWithStartTime, runs);
  console.log(`With start time average time: ${duration} ms`);
  const ruleWithoutStartTime = `freq=DAILY\ntzid=Europe/Helsinki\ninterval=1\nbyweekday=MO,TU,WE,TH,FR,SA,SU\nbyhour=2\nbyminute=0\nbysecond=0`;
  duration = await benchmark(rruleGenerateDates, ruleWithoutStartTime, runs);
  console.log(`Without start time average time: ${duration} ms`);

  console.log("Same rule but difference in betweens after parameter:");
  duration = await benchmark(rruleGenerateDates, ruleWithStartTime, runs, new Date("2060-09-30"));
  console.log(`2060 after date average time: ${duration} ms`);
  duration = await benchmark(rruleGenerateDates, ruleWithStartTime, runs, new Date("2021-09-30"));
  console.log(`2021 after date average time: ${duration} ms`);
}
//With start time average time: 681.4148922994733 ms
//Without start time average time: 0.735097499936819 ms
//Same rule but difference in betweens after parameter:
//2060 after date average time: 8109.418215399981 ms
//2021 after date average time: 750.5864615000785 ms
runBenchmark(10);
SijmenHuizenga commented 1 month ago

In our case, most of the time the local timezone is equal to the timezone in the rrule string. So we got a big performance increase after stripping any mentions of the local timezone.

function removeRedundantTzId(rrule: string) {
  // workaround for this bug: https://github.com/jkbrzt/rrule/issues/580
  const localTzid = Intl.DateTimeFormat().resolvedOptions().timeZone;
  return rrule.replace(`;TZID=${localTzid}`, "");
}

edit: In the end, this workaround was not enough for us. We switched to rschedule and achieved a huge performance increase. Calculating all events in a certain duration went from ~700ms to ~20ms.