Skip to content

js: Lazily initialise optional QuickJS intrinsics - #8397

Closed
Eddy Ashton (eddyashton) wants to merge 18 commits into
mainfrom
quickjs-lazy-intrinsic-constructors
Closed

Eddy Ashton (eddyashton) wants to merge 18 commits into
mainfrom
quickjs-lazy-intrinsic-constructors

Conversation

@eddyashton

Copy link
Copy Markdown
Member

Summary

Stacked on #8393, which adds the interpreter lifecycle benchmark used to measure this change.

Defer construction of the context-local constructor/prototype families for:

  • Date
  • Map
  • Set
  • WeakMap
  • WeakSet

JS_NewContext() installs normal writable/configurable auto-initialized globals. Each family is still created fresh inside that context on first access; no runtime, context, prototype, module, or JavaScript state is reused across requests.

The change is carried as a numbered local QuickJS patch rather than modifying the exported upstream snapshot.

Motivation and performance

These constructor families account for approximately 11 us of fresh QuickJS context creation even when an application never accesses them.

Ten matched alternating local microbenchmark pairs showed:

Benchmark Paired median change
QuickJS standard context lifecycle -12.4%
CCF core Context lifecycle -14.3%
CCF CommonContext lifecycle -11.6%
Registry interpreter factory -11.4%
Fresh compile and module evaluation -11.1%
Fresh compile/evaluate/direct call -10.6%

A pessimistic handler which constructs and uses all five deferred families is statistically neutral against eager initialization (paired median -0.9%, range -3.3% to +1.3%), so the work is deferred rather than multiplied.

Local Basic JS e2e results are directionally positive but noisy. Across eight paired runs at four task threads, 7/8 improved, with a paired median of +3.5% throughput and a range from -1.7% to +10.2%. Reliable CI performance results should be the deciding evidence for the end-to-end impact.

Safety and compatibility

The patch preserves descriptors, constructor identity, subclassing, iterators, native Date construction, object deserialization, and the public eager JS_AddIntrinsicDate/JS_AddIntrinsicMapSet APIs.

Auto-initialization remains retryable after allocation failure. Partial prototype state is cleared before retry, and native Date retains the original constructor independently of script mutation of Date.prototype.constructor.

Coverage includes:

  • first use and constructor metadata;
  • overwrite/delete before materialization;
  • subclassing and Map/Set iterator prototypes;
  • native and deserialized Date identity;
  • first-access and native-Date OOM followed by retry;
  • public eager intrinsic APIs from a raw context.

Validation

  • js_test
  • js_policy_test
  • js_interpreter_bench
  • local QuickJS patches apply in order with --fuzz=0
  • C++/CMake/Prettier formatting
  • ASCII and copyright checks

Add stable microbenchmarks for bare QuickJS and CCF interpreter construction, fresh module invocation, and cache lookup paths. Publish the key lifecycle metrics through the existing benchmark converter.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the new internal benchmark out of the general performance overview.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Defer Date and Map/Set constructor families until first access while retaining fresh runtimes and contexts. Keep failed initialization retryable, preserve native Date identity, and cover OOM, serialization, mutation, and eager API compatibility.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Record the new benchmark in the no_bucket snapshot so the inventory matches its intentional CMake registration.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the generated CTest bucket snapshot in sync with the benchmark introduced by the stacked base PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Description

Comparing 5 available runs from this branch (#8397) against the trend of the last 30 main runs.

Each chart plots every benchmark as an axis, with values normalized so 100 is the EWMA baseline of recent main runs, using a 7-run half-life. The 5 orange branch lines run from the oldest (faintest) to the latest (darkest and thickest); the darker blue band is the main baseline +/- 1 std dev and the lighter blue band around it is +/- 2 std dev.

Axis labels show the latest branch value and its difference from the main EWMA baseline, where 0% is on the baseline. They are coloured green where the latest run improves on the baseline, red where it regresses, and grey where the difference is within one std dev of the baseline (within noise). Higher is better for throughput and rate, lower for latency and memory.

A benchmark which does not exist on main yet has no baseline of its own, so its earliest available run from this branch is used as its reference and its band is measured across this branch's runs. Its axis is normalized, scaled and coloured like any other, but the comparison is against this branch rather than against main.

Throughput (tx/s)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(7){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic Blocking 100ms: 3,088 tx/s ▬ 0%"]
  axis b1["Basic Blocking 20ms: 15,348 tx/s ▬ 0%"]
  axis b2["Basic Blocking 2ms: 52,887 tx/s ▬ +1%"]
  axis b3["Basic JS: 17,173 tx/s ▲ 6%"]
  axis b4["Historical Queries: 1,035,958 tx/s ▬ +8%"]
  axis b5["L…g Certificate Blocking: 29,421 tx/s ▬ 0%"]
  axis b6["Logging JWT Blocking: 15,378 tx/s ▬ 0%"]
  curve stddev2_high["main EWMA + 2 std dev"]{100.37, 104.89, 107.68, 107.92, 125.96, 100.51, 100.30}
  curve stddev1_high["main EWMA + 1 std dev"]{100.18, 102.44, 103.84, 103.96, 112.98, 100.26, 100.15}
  curve stddev1_low["main EWMA - 1 std dev"]{99.82, 97.56, 96.16, 96.04, 87.02, 99.74, 99.85}
  curve stddev2_low["main EWMA - 2 std dev"]{99.63, 95.11, 92.32, 92.08, 74.04, 99.49, 99.70}
  curve branch_0["#8397 (4 runs earlier)"]{99.91, 100.38, 100.81, 102.86, 105.83, 100.00, 100.00}
  curve branch_1["#8397 (3 runs earlier)"]{100.03, 100.49, 95.92, 100.91, 97.88, 99.95, 99.98}
  curve branch_2["#8397 (2 runs earlier)"]{100.07, 100.39, 96.68, 108.05, 106.05, 100.12, 99.80}
  curve branch_3["#8397 (1 run earlier)"]{99.84, 100.48, 97.18, 104.29, 104.79, 99.80, 100.00}
  curve branch_4["#8397"]{99.88, 100.37, 100.73, 106.20, 108.34, 99.69, 100.08}
  graticule polygon
  max 145
  min 55
  ticks 0
  showLegend false
Loading

Latency (ms)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(7){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic Blocking 100ms: 98 ms ▼ 1%"]
  axis b1["Basic Blocking 20ms: 19 ms ▬ 0%"]
  axis b2["Basic Blocking 2ms: 5 ms ▬ 0%"]
  axis b3["Basic JS: 18 ms ▼ 4%"]
  axis b4["Historical Queries: 28 ms ▬ -12%"]
  axis b5["Logging Certificate Blocking: 19 ms ▬ 0%"]
  axis b6["Logging JWT Blocking: 19 ms ▬ 0%"]
  curve stddev2_high["main EWMA + 2 std dev"]{100.61, 100.00, 107.16, 107.22, 134.71, 100.00, 100.00}
  curve stddev1_high["main EWMA + 1 std dev"]{100.30, 100.00, 103.58, 103.61, 117.35, 100.00, 100.00}
  curve stddev1_low["main EWMA - 1 std dev"]{99.70, 100.00, 96.42, 96.39, 82.65, 100.00, 100.00}
  curve stddev2_low["main EWMA - 2 std dev"]{99.39, 100.00, 92.84, 92.78, 65.29, 100.00, 100.00}
  curve branch_0["#8397 (4 runs earlier)"]{100.11, 100.00, 99.65, 95.59, 97.02, 100.00, 100.00}
  curve branch_1["#8397 (3 runs earlier)"]{100.11, 100.00, 99.65, 100.90, 100.15, 100.00, 100.00}
  curve branch_2["#8397 (2 runs earlier)"]{100.11, 100.00, 99.65, 90.28, 90.76, 100.00, 100.00}
  curve branch_3["#8397 (1 run earlier)"]{100.11, 100.00, 99.65, 95.59, 90.76, 100.00, 100.00}
  curve branch_4["#8397"]{99.10, 100.00, 99.65, 95.59, 87.63, 100.00, 100.00}
  graticule polygon
  max 160
  min 40
  ticks 0
  showLegend false
Loading

Memory (bytes)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(7){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic Blocking 100ms: 89.1 MiB ▬ +1%"]
  axis b1["Basic Blocking 20ms: 88.6 MiB ▬ 0%"]
  axis b2["Basic Blocking 2ms: 90.8 MiB ▬ 0%"]
  axis b3["Basic JS: 96 MiB ▬ -1%"]
  axis b4["Historical Queries: 151 MiB ▬ +1%"]
  axis b5["Logging Certificate Blocking: 114 MiB ▬ 0%"]
  axis b6["Logging JWT Blocking: 87.2 MiB ▬ 0%"]
  curve stddev2_high["main EWMA + 2 std dev"]{104.36, 104.43, 104.09, 101.97, 101.70, 103.70, 106.02}
  curve stddev1_high["main EWMA + 1 std dev"]{102.18, 102.21, 102.05, 100.98, 100.85, 101.85, 103.01}
  curve stddev1_low["main EWMA - 1 std dev"]{97.82, 97.79, 97.95, 99.02, 99.15, 98.15, 96.99}
  curve stddev2_low["main EWMA - 2 std dev"]{95.64, 95.57, 95.91, 98.03, 98.30, 96.30, 93.98}
  curve branch_0["#8397 (4 runs earlier)"]{100.62, 98.00, 98.22, 98.62, 99.76, 100.23, 99.05}
  curve branch_1["#8397 (3 runs earlier)"]{99.30, 98.96, 99.70, 98.43, 99.87, 98.38, 101.22}
  curve branch_2["#8397 (2 runs earlier)"]{101.11, 98.88, 99.78, 100.90, 100.00, 100.67, 98.51}
  curve branch_3["#8397 (1 run earlier)"]{99.06, 100.02, 99.88, 99.62, 99.74, 99.24, 99.01}
  curve branch_4["#8397"]{100.84, 99.90, 99.60, 99.32, 100.51, 100.08, 100.16}
  graticule polygon
  max 111
  min 89
  ticks 0
  showLegend false
Loading

Rate (ops/s)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.20!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.30!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-7{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-8{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(2){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(3){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(7){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(8){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(9){fill:#2DA44E!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    cScale8: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["CCF c…n c…t lifecycle: 24,063 ops/s ▲ 16%"]
  axis b1["CCF fresh JS invocation: 21,952 ops/s ▲ 15%"]
  axis b2["CHAMP get: 66,392,194 ops/s ▲ 4%"]
  axis b3["CHAMP put: 8,081,382 ops/s ▬ 0%"]
  axis b4["KV deserialisation: 2,349,072 ops/s ▬ 0%"]
  axis b5["KV serialisation: 2,029,633 ops/s ▬ +1%"]
  axis b6["KV s…t deserialisation: 6,287 ops/s ▬ +1%"]
  axis b7["KV snapshot serialisation: 4,987 ops/s ▲ 4%"]
  axis b8["Q…S s…d c…t lifecycle: 30,059 ops/s ▲ 19%"]
  curve stddev2_high["main EWMA + 2 std dev"]{104.19, 104.06, 104.43, 105.46, 104.68, 104.13, 104.77, 105.65, 104.58}
  curve stddev1_high["main EWMA + 1 std dev"]{102.10, 102.03, 102.22, 102.73, 102.34, 102.06, 102.39, 102.83, 102.29}
  curve stddev1_low["main EWMA - 1 std dev"]{97.90, 97.97, 97.78, 97.27, 97.66, 97.94, 97.61, 97.17, 97.71}
  curve stddev2_low["main EWMA - 2 std dev"]{95.81, 95.94, 95.57, 94.54, 95.32, 95.87, 95.23, 94.35, 95.42}
  curve branch_0["#8397 (4 runs earlier)"]{116.29, 113.45, 102.99, 99.81, 103.49, 98.82, 102.06, 98.18, 118.65}
  curve branch_1["#8397 (3 runs earlier)"]{105.87, 104.01, 93.82, 94.36, 99.61, 100.20, 93.41, 95.96, 107.04}
  curve branch_2["#8397 (2 runs earlier)"]{116.11, 113.26, 102.85, 93.11, 102.49, 102.07, 101.58, 99.09, 117.86}
  curve branch_3["#8397 (1 run earlier)"]{116.12, 115.30, 99.97, 97.90, 101.49, 102.07, 100.63, 97.79, 117.51}
  curve branch_4["#8397"]{115.91, 115.14, 104.35, 99.88, 99.82, 100.62, 100.94, 104.37, 118.51}
  graticule polygon
  max 128
  min 84
  ticks 0
  showLegend false
Loading

Keep the deserialized Date constructor in a wrapper bound to its own QuickJS context, so Debug leak checks free it through the correct runtime.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Base automatically changed from js-interpreter-lifecycle-benchmark to main September 18, 2026 10:06
Apply the repository clang-format layout required by VMSS Virtual A checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@eddyashton
Eddy Ashton (eddyashton) marked this pull request as ready for review September 18, 2026 14:23
Copilot AI lite review requested due to automatic review settings September 18, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The moderate Date initialization failure-path issue must be fixed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR lazily initializes optional QuickJS intrinsic families per context to reduce interpreter startup overhead.

Changes:

  • Defers Date, Map, Set, WeakMap, and WeakSet initialization.
  • Adds regression, OOM-retry, and benchmark coverage.
  • Applies and documents the numbered QuickJS patch.

Review finding: the lazy Date path can retain partial prototype state after constructor allocation failure, causing later access to produce undefined instead of retrying. Cleanup or routing through JS_EnsureIntrinsicDate is required at the noted lines.

File summaries
File Description
src/js/test/js.cpp Tests lazy intrinsic behavior and retry paths.
src/js/test/interpreter_bench.cpp Adds deferred-intrinsic lifecycle benchmarks.
cmake/quickjs.cmake Applies the new QuickJS patch.
3rdparty/patches/quickjs-2026-06-04/README.md Documents the patch and coverage.
3rdparty/patches/quickjs-2026-06-04/0003-lazy-intrinsic-constructors.patch Implements lazy intrinsic construction.
.gitattributes Configures patch whitespace handling.
Review details

Suppressed comments (1)

3rdparty/patches/quickjs-2026-06-04/0003-lazy-intrinsic-constructors.patch:332

  • After a family is successfully built but JS_AutoInitProperty fails to allocate its global var-ref, the auto-init entry remains and class_proto is already non-null. A retry therefore rebuilds the family here; JS_NewCConstructor overwrites the existing prototype without freeing it, so repeated retries leak the old family/iterator and a failed replacement can leave cleanup inconsistent. Reuse the existing prototype's constructor when it is already initialized, or make replacement transactional.
+        value = JS_CreateIntrinsicMapSet(
+            ctx,
+            intrinsic - JS_LAZY_INTRINSIC_MAP,
+            JS_NEW_CTOR_NO_GLOBAL);
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Ensure failed Date auto-initialization clears partial prototype state before the auto-init property is retried.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@eddyashton

Copy link
Copy Markdown
Member Author

After a few days' consideration, I think this is too risky. It's a very big patch, changing fundamental behaviour, for a throughput bump that isn't currently needed. We should keep it in the back pocket as a potential JS perf improvement, but only merge with stronger justification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants