Repository: moment/luxon
Pull request #1579 (2024-01-22 16:20:09)

Perf: Cache ts offset guesses for quickDT
_This is part of a series of PRs based on performance work we have done to
improve a use-case involving parsing/formatting hundreds of thousands of dates
where luxon was the bottleneck._

_This includes the commit from https://github.com/moment/luxon/pull/1574 to establish the benchmark_

Previously we guessed by calling zone.offset(tsNow) on every
construction of a date from date parts. This caches that result instead,
saving a call to offset. The tests verify that this works across DST boundaries when the cache holds an offset for the previous DST state from now.

For processes that live across a DST boundary and mostly handle current date times, this will mean the guess is wrong and we'll have to re-guess. Since we're caching one call to offset, this should be performance neutral to current behavior. Otherwise, we save a call.

Benchmark Comparison (`name | before | after | after/before`):
```
DateTime.local with numbers and zone | 50,913 ±0.18% | 66,168 ±0.25% | 1.3x
```

schleyfox: /easycla
icambron: Looks pretty good. Just requested a few changes
schleyfox: Changed, thanks!
icambron: @schleyfox if you fix the conflicts, I'll merge this
schleyfox: @icambron conflicts fixed. thanks.

Patch:
diff --git a/src/datetime.js b/src/datetime.js
index 5314a9b3..09a389e7 100644
--- a/src/datetime.js
+++ b/src/datetime.js
@@ -370,6 +370,36 @@ function normalizeUnitWithLocalWeeks(unit) {
   }
 }
 
+// cache offsets for zones based on the current timestamp when this function is
+// first called. When we are handling a datetime from components like (year,
+// month, day, hour) in a time zone, we need a guess about what the timezone
+// offset is so that we can convert into a UTC timestamp. One way is to find the
+// offset of now in the zone. The actual date may have a different offset (for
+// example, if we handle a date in June while we're in December in a zone that
+// observes DST), but we can check and adjust that.
+//
+// When handling many dates, calculating the offset for now every time is
+// expensive. It's just a guess, so we can cache the offset to use even if we
+// are right on a time change boundary (we'll just correct in the other
+// direction). Using a timestamp from first read is a slight optimization for
+// handling dates close to the current date, since those dates will usually be
+// in the same offset (we could set the timestamp statically, instead). We use a
+// single timestamp for all zones to make things a bit more predictable.
+//
+// This is safe for quickDT (used by local() and utc()) because we don't fill in
+// higher-order units from tsNow (as we do in fromObject, this requires that
+// offset is calculated from tsNow).
+function guessOffsetForZone(zone) {
+  if (!zoneOffsetGuessCache[zone]) {
+    if (zoneOffsetTs === undefined) {
+      zoneOffsetTs = Settings.now();
+    }
+
+    zoneOffsetGuessCache[zone] = zone.offset(zoneOffsetTs);
+  }
+  return zoneOffsetGuessCache[zone];
+}
+
 // this is a dumbed down version of fromObject() that runs about 60% faster
 // but doesn't do any validation, makes a bunch of assumptions about what units
 // are present, and so on.
@@ -379,8 +409,7 @@ function quickDT(obj, opts) {
     return DateTime.invalid(unsupportedZone(zone));
   }
 
-  const loc = Locale.fromObject(opts),
-    tsNow = Settings.now();
+  const loc = Locale.fromObject(opts);
 
   let ts, o;
 
@@ -397,10 +426,10 @@ function quickDT(obj, opts) {
       return DateTime.invalid(invalid);
     }
 
-    const offsetProvis = zone.offset(tsNow);
+    const offsetProvis = guessOffsetForZone(zone);
     [ts, o] = objToTS(obj, offsetProvis, zone);
   } else {
-    ts = tsNow;
+    ts = Settings.now();
   }
 
   return new DateTime({ ts, zone, loc, o });
@@ -448,6 +477,18 @@ function lastOpts(argList) {
   return [opts, args];
 }
 
+/**
+ * Timestamp to use for cached zone offset guesses (exposed for test)
+ */
+let zoneOffsetTs;
+/**
+ * Cache for zone offset guesses (exposed for test).
+ *
+ * This optimizes quickDT via guessOffsetForZone to avoid repeated calls of
+ * zone.offset().
+ */
+let zoneOffsetGuessCache = {};
+
 /**
  * A DateTime is an immutable data structure representing a specific date and time and accompanying methods. It contains class and instance methods for creating, parsing, interrogating, transforming, and formatting them.
  *
@@ -1000,6 +1041,11 @@ export default class DateTime {
     return expanded.map((t) => t.val).join("");
   }
 
+  static resetCache() {
+    zoneOffsetTs = undefined;
+    zoneOffsetGuessCache = {};
+  }
+
   // INFO
 
   /**
diff --git a/src/settings.js b/src/settings.js
index f109708d..5356dd67 100644
--- a/src/settings.js
+++ b/src/settings.js
@@ -1,6 +1,7 @@
 import SystemZone from "./zones/systemZone.js";
 import IANAZone from "./zones/IANAZone.js";
 import Locale from "./impl/locale.js";
+import DateTime from "./datetime.js";
 
 import { normalizeZone } from "./impl/zoneUtil.js";
 import { validateWeekSettings } from "./impl/util.js";
@@ -130,7 +131,7 @@ export default class Settings {
   }
 
   /**
-   * Get the cutoff year after which a string encoding a year as two digits is interpreted to occur in the current century.
+   * Get the cutoff year for whether a 2-digit year string is interpreted in the current or previous century. Numbers higher than the cutoff will be considered to mean 19xx and numbers lower or equal to the cutoff will be considered 20xx.
    * @type {number}
    */
   static get twoDigitCutoffYear() {
@@ -138,10 +139,11 @@ export default class Settings {
   }
 
   /**
-   * Set the cutoff year after which a string encoding a year as two digits is interpreted to occur in the current century.
+   * Set the cutoff year for whether a 2-digit year string is interpreted in the current or previous century. Numbers higher than the cutoff will be considered to mean 19xx and numbers lower or equal to the cutoff will be considered 20xx.
    * @type {number}
-   * @example Settings.twoDigitCutoffYear = 0 // cut-off year is 0, so all 'yy' are interpreted as current century
-   * @example Settings.twoDigitCutoffYear = 50 // '49' -> 1949; '50' -> 2050
+   * @example Settings.twoDigitCutoffYear = 0 // all 'yy' are interpreted as 20th century
+   * @example Settings.twoDigitCutoffYear = 99 // all 'yy' are interpreted as 21st century
+   * @example Settings.twoDigitCutoffYear = 50 // '49' -> 2049; '50' -> 1950
    * @example Settings.twoDigitCutoffYear = 1950 // interpreted as 50
    * @example Settings.twoDigitCutoffYear = 2050 // ALSO interpreted as 50
    */
@@ -172,6 +174,7 @@ export default class Settings {
   static resetCaches() {
     Locale.resetCache();
     IANAZone.resetCache();
+    DateTime.resetCache();
     resetDigitRegexCache();
   }
 }


Test patch:
diff --git a/test/datetime/dst.test.js b/test/datetime/dst.test.js
index b28aa051..8ab5d77a 100644
--- a/test/datetime/dst.test.js
+++ b/test/datetime/dst.test.js
@@ -2,111 +2,191 @@
 
 import { DateTime, Settings } from "../../src/luxon";
 
-const local = (year, month, day, hour) =>
-  DateTime.fromObject({ year, month, day, hour }, { zone: "America/New_York" });
-
-test("Hole dates are bumped forward", () => {
-  const d = local(2017, 3, 12, 2);
-  expect(d.hour).toBe(3);
-  expect(d.offset).toBe(-4 * 60);
-});
-
-// this is questionable behavior, but I wanted to document it
-test("Ambiguous dates pick the one with the current offset", () => {
-  const oldSettings = Settings.now;
-  try {
-    Settings.now = () => 1495653314595; // May 24, 2017
-    let d = local(2017, 11, 5, 1);
-    expect(d.hour).toBe(1);
-    expect(d.offset).toBe(-4 * 60);
-
-    Settings.now = () => 1484456400000; // Jan 15, 2017
-    d = local(2017, 11, 5, 1);
-    expect(d.hour).toBe(1);
-    expect(d.offset).toBe(-5 * 60);
-  } finally {
-    Settings.now = oldSettings;
-  }
-});
-
-test("Adding an hour to land on the Spring Forward springs forward", () => {
-  const d = local(2017, 3, 12, 1).plus({ hour: 1 });
-  expect(d.hour).toBe(3);
-  expect(d.offset).toBe(-4 * 60);
-});
-
-test("Subtracting an hour to land on the Spring Forward springs forward", () => {
-  const d = local(2017, 3, 12, 3).minus({ hour: 1 });
-  expect(d.hour).toBe(1);
-  expect(d.offset).toBe(-5 * 60);
-});
-
-test("Adding an hour to land on the Fall Back falls back", () => {
-  const d = local(2017, 11, 5, 0).plus({ hour: 2 });
-  expect(d.hour).toBe(1);
-  expect(d.offset).toBe(-5 * 60);
-});
-
-test("Subtracting an hour to land on the Fall Back falls back", () => {
-  let d = local(2017, 11, 5, 3).minus({ hour: 2 });
-  expect(d.hour).toBe(1);
-  expect(d.offset).toBe(-5 * 60);
-
-  d = d.minus({ hour: 1 });
-  expect(d.hour).toBe(1);
-  expect(d.offset).toBe(-4 * 60);
-});
-
-test("Changing a calendar date to land on a hole bumps forward", () => {
-  let d = local(2017, 3, 11, 2).plus({ day: 1 });
-  expect(d.hour).toBe(3);
-  expect(d.offset).toBe(-4 * 60);
-
-  d = local(2017, 3, 13, 2).minus({ day: 1 });
-  expect(d.hour).toBe(3);
-  expect(d.offset).toBe(-4 * 60);
-});
-
-test("Changing a calendar date to land on an ambiguous time chooses the closest one", () => {
-  let d = local(2017, 11, 4, 1).plus({ day: 1 });
-  expect(d.hour).toBe(1);
-  expect(d.offset).toBe(-4 * 60);
-
-  d = local(2017, 11, 6, 1).minus({ day: 1 });
-  expect(d.hour).toBe(1);
-  expect(d.offset).toBe(-5 * 60);
-});
-
-test("Start of a 0:00->1:00 DST day is 1:00", () => {
-  const d = DateTime.fromObject(
-    {
-      year: 2017,
-      month: 10,
-      day: 15,
-    },
-    {
-      zone: "America/Sao_Paulo",
+const dateTimeConstructors = {
+  fromObject: (year, month, day, hour) =>
+    DateTime.fromObject({ year, month, day, hour }, { zone: "America/New_York" }),
+  local: (year, month, day, hour) =>
+    DateTime.local(year, month, day, hour, { zone: "America/New_York" }),
+};
+
+for (const [name, local] of Object.entries(dateTimeConstructors)) {
+  describe(`DateTime.${name}`, () => {
+    test("Hole dates are bumped forward", () => {
+      const d = local(2017, 3, 12, 2);
+      expect(d.hour).toBe(3);
+      expect(d.offset).toBe(-4 * 60);
+    });
+
+    if (name == "fromObject") {
+      // this is questionable behavior, but I wanted to document it
+      test("Ambiguous dates pick the one with the current offset", () => {
+        const oldSettings = Settings.now;
+        try {
+          Settings.now = () => 1495653314595; // May 24, 2017
+          let d = local(2017, 11, 5, 1);
+          expect(d.hour).toBe(1);
+          expect(d.offset).toBe(-4 * 60);
+
+          Settings.now = () => 1484456400000; // Jan 15, 2017
+          d = local(2017, 11, 5, 1);
+          expect(d.hour).toBe(1);
+          expect(d.offset).toBe(-5 * 60);
+        } finally {
+          Settings.now = oldSettings;
+        }
+      });
+    } else {
+      test("Ambiguous dates pick the one with the cached offset", () => {
+        const oldSettings = Settings.now;
+        try {
+          Settings.resetCaches();
+          Settings.now = () => 1495653314595; // May 24, 2017
+          let d = local(2017, 11, 5, 1);
+          expect(d.hour).toBe(1);
+          expect(d.offset).toBe(-4 * 60);
+
+          Settings.now = () => 1484456400000; // Jan 15, 2017
+          d = local(2017, 11, 5, 1);
+          expect(d.hour).toBe(1);
+          expect(d.offset).toBe(-4 * 60);
+
+          Settings.resetCaches();
+
+          Settings.now = () => 1484456400000; // Jan 15, 2017
+          d = local(2017, 11, 5, 1);
+          expect(d.hour).toBe(1);
+          expect(d.offset).toBe(-5 * 60);
+
+          Settings.now = () => 1495653314595; // May 24, 2017
+          d = local(2017, 11, 5, 1);
+          expect(d.hour).toBe(1);
+          expect(d.offset).toBe(-5 * 60);
+        } finally {
+          Settings.now = oldSettings;
+        }
+      });
     }
-  ).startOf("day");
-  expect(d.day).toBe(15);
-  expect(d.hour).toBe(1);
-  expect(d.minute).toBe(0);
-  expect(d.second).toBe(0);
-});
 
-test("End of a 0:00->1:00 DST day is 23:59", () => {
-  const d = DateTime.fromObject(
-    {
-      year: 2017,
-      month: 10,
-      day: 15,
-    },
-    {
-      zone: "America/Sao_Paulo",
+    test("Adding an hour to land on the Spring Forward springs forward", () => {
+      const d = local(2017, 3, 12, 1).plus({ hour: 1 });
+      expect(d.hour).toBe(3);
+      expect(d.offset).toBe(-4 * 60);
+    });
+
+    test("Subtracting an hour to land on the Spring Forward springs forward", () => {
+      const d = local(2017, 3, 12, 3).minus({ hour: 1 });
+      expect(d.hour).toBe(1);
+      expect(d.offset).toBe(-5 * 60);
+    });
+
+    test("Adding an hour to land on the Fall Back falls back", () => {
+      const d = local(2017, 11, 5, 0).plus({ hour: 2 });
+      expect(d.hour).toBe(1);
+      expect(d.offset).toBe(-5 * 60);
+    });
+
+    test("Subtracting an hour to land on the Fall Back falls back", () => {
+      let d = local(2017, 11, 5, 3).minus({ hour: 2 });
+      expect(d.hour).toBe(1);
+      expect(d.offset).toBe(-5 * 60);
+
+      d = d.minus({ hour: 1 });
+      expect(d.hour).toBe(1);
+      expect(d.offset).toBe(-4 * 60);
+    });
+
+    test("Changing a calendar date to land on a hole bumps forward", () => {
+      let d = local(2017, 3, 11, 2).plus({ day: 1 });
+      expect(d.hour).toBe(3);
+      expect(d.offset).toBe(-4 * 60);
+
+      d = local(2017, 3, 13, 2).minus({ day: 1 });
+      expect(d.hour).toBe(3);
+      expect(d.offset).toBe(-4 * 60);
+    });
+
+    test("Changing a calendar date to land on an ambiguous time chooses the closest one", () => {
+      let d = local(2017, 11, 4, 1).plus({ day: 1 });
+      expect(d.hour).toBe(1);
+      expect(d.offset).toBe(-4 * 60);
+
+      d = local(2017, 11, 6, 1).minus({ day: 1 });
+      expect(d.hour).toBe(1);
+      expect(d.offset).toBe(-5 * 60);
+    });
+
+    test("Start of a 0:00->1:00 DST day is 1:00", () => {
+      const d = DateTime.fromObject(
+        {
+          year: 2017,
+          month: 10,
+          day: 15,
+        },
+        {
+          zone: "America/Sao_Paulo",
+        }
+      ).startOf("day");
+      expect(d.day).toBe(15);
+      expect(d.hour).toBe(1);
+      expect(d.minute).toBe(0);
+      expect(d.second).toBe(0);
+    });
+
+    test("End of a 0:00->1:00 DST day is 23:59", () => {
+      const d = DateTime.fromObject(
+        {
+          year: 2017,
+          month: 10,
+          day: 15,
+        },
+        {
+          zone: "America/Sao_Paulo",
+        }
+      ).endOf("day");
+      expect(d.day).toBe(15);
+      expect(d.hour).toBe(23);
+      expect(d.minute).toBe(59);
+      expect(d.second).toBe(59);
+    });
+  });
+}
+
+describe("DateTime.local() with offset caching", () => {
+  const edtTs = 1495653314000; // May 24, 2017 15:15:14 -0400
+  const estTs = 1484456400000; // Jan 15, 2017 00:00 -0500
+
+  const edtDate = [2017, 5, 24, 15, 15, 14, 0];
+  const estDate = [2017, 1, 15, 0, 0, 0, 0];
+
+  const timestamps = { EDT: edtTs, EST: estTs };
+  const dates = { EDT: edtDate, EST: estDate };
+  const zoneObj = { zone: "America/New_York" };
+
+  for (const [cacheName, cacheTs] of Object.entries(timestamps)) {
+    for (const [nowName, nowTs] of Object.entries(timestamps)) {
+      for (const [dateName, date] of Object.entries(dates)) {
+        test(`cache = ${cacheName}, now = ${nowName}, date = ${dateName}`, () => {
+          const oldSettings = Settings.now;
+          try {
+            Settings.now = () => cacheTs;
+            Settings.resetCaches();
+            // load cache
+            DateTime.local(2020, 1, 1, 0, zoneObj);
+
+            Settings.now = () => nowTs;
+            const dt = DateTime.local(...date, zoneObj);
+            expect(dt.toMillis()).toBe(timestamps[dateName]);
+            expect(dt.year).toBe(date[0]);
+            expect(dt.month).toBe(date[1]);
+            expect(dt.day).toBe(date[2]);
+            expect(dt.hour).toBe(date[3]);
+            expect(dt.minute).toBe(date[4]);
+            expect(dt.second).toBe(date[5]);
+          } finally {
+            Settings.now = oldSettings;
+          }
+        });
+      }
     }
-  ).endOf("day");
-  expect(d.day).toBe(15);
-  expect(d.hour).toBe(23);
-  expect(d.minute).toBe(59);
-  expect(d.second).toBe(59);
+  }
 });
diff --git a/test/datetime/regexParse.test.js b/test/datetime/regexParse.test.js
index db8ee530..db84b968 100644
--- a/test/datetime/regexParse.test.js
+++ b/test/datetime/regexParse.test.js
@@ -1,6 +1,7 @@
 /* global test expect */
 
 import { DateTime } from "../../src/luxon";
+import { withNow } from "../helpers";
 
 //------
 // .fromISO
@@ -624,6 +625,28 @@ test("DateTime.fromISO() accepts extended zones on bare times", () => {
   });
 });
 
+withNow(
+  "DateTime.fromISO() accepts extended zones on bare times when UTC and zone are in different days",
+  DateTime.fromISO("2023-11-20T23:30:00.000Z"),
+  () => {
+    const { year, month, day } = DateTime.now().setZone("Europe/Paris");
+    let dt = DateTime.fromISO("10:23:54[Europe/Paris]", {
+      setZone: true,
+    });
+    expect(dt.isValid).toBe(true);
+    expect(dt.zoneName).toBe("Europe/Paris");
+    expect(dt.toObject()).toEqual({
+      year,
+      month,
+      day,
+      hour: 10,
+      minute: 23,
+      second: 54,
+      millisecond: 0,
+    });
+  }
+);
+
 test("DateTime.fromISO() accepts some technically incorrect stuff", () => {
   // these are formats that aren't technically valid but we parse anyway.
   // Testing them more to document them than anything else