From 20cf82deb2a10014532166f71ffa826c1eb55fec Mon Sep 17 00:00:00 2001
From: Gregg Tavares <github@greggman.com>
Date: Thu, 14 Nov 2024 20:15:30 -0800
Subject: [PATCH 1/3] Cleanup timestamp-query example

This was a great example. I started this change as just
removing a `console.log` that seemed like it shouldn't
be there but then I noticed the code didn't actually
handle the case when 'timestamp-query' doesn't exist
and ended up making a bunch of changes.

I didn't mean to step on anyone's toes. Here's the
change though.

1. Remove `console.log` from timestamp query sample

   This was just filling all of memory with strings.
   Probably not a big deal since no one is likely to
   run the sample for hours but still....

   Maybe add a `<pre>` spot that holds the last N queries
   if you want to see a history?

2. no need for `hasOngoingTimestampReadback`

   `GPUBuffer` already has `mapState` which tells you this state
   so no need to track it separately.

3. There's no need for timestampCount

   It's already on `GPUQuerySet.count`

4. code didn't check if timestamps are supported before adding
   `timestamp` section to the render pass so the sample got a validation
   error if timestamp-query was not avaialble.

   Note: you can test this with the webgpu-dev-extension. Type
   'timestamp-query' into the "Block Feature" field and
   reload the page.

5. Refactor not to use `readAsync`

   This seems like a problematic API. You call `readAsync`
   and if the map is pending then your callback is ignored.
   Ideally, if a callback was the right API design, then
   every callback you pass should get called and if it's pending
   then it would need to add your callback to a list of callbacks
   to be called when it's ready.

   Further, the timestamps passed back are only valid
   during the callback. If you tried to keep them they'd
   magically disappear as soon as your callback exited
   since they're directly the mapped buffer which will be
   unmapped as soon as you exit.

   Further, I'd argue for this sample at least, all you want is
   the last timing.

   So, changed the API to just give you the last times.
   No callbacks.

6. It's not clear what TimestampQueryManager was really managing.

   It wasn't adding the timestamps for you.
   You had to add them yourself and pull out the timestamp
   query object from internal fields inside TimestampQueryManager.

   Further, because you were adding them yourself you'd
   have to check yourself, if queries were supported then
   add them.

   Then, beacuse you were adding them yourself it was up to you
   to choose start and ends indices.

   It was also passing out BigInt but BitInt is hard to use correctly.
   I'd argue you don't need it. IIUC, Number.MAX_SAFE_INTEGER is 104 days
   of nanoseconds so it's fine to convert and give the user
   something easy to work with.

   So, I changed it to take a pairId and then use that
   to set both start and end. That way it can auto-subtract the
   pairs and give you Number. The user doesn't have to deal with
   BigInt.

Other: I considered even getting rid of pairId and have `addTimestampWrite`
just auto increment the indices to use. It would then reset to 0 when you
call `update`. With that, it could easily add up the times
and give you the total time across passes.

That way you wouldn't have to manage the pairIds yourself. On the otherhand,
if you wanted individual pass timings you need to know which pass got which
pairId. It could pass the pairId back from `addTimestamp` but I was less sure
about that change.

Anyway, I hope this is considered an improvement.
---
 .../timestampQuery/TimestampQueryManager.ts   | 64 ++++++++++++-------
 sample/timestampQuery/main.ts                 | 39 ++++-------
 2 files changed, 51 insertions(+), 52 deletions(-)

diff --git a/sample/timestampQuery/TimestampQueryManager.ts b/sample/timestampQuery/TimestampQueryManager.ts
index 46696418..54be3c42 100644
--- a/sample/timestampQuery/TimestampQueryManager.ts
+++ b/sample/timestampQuery/TimestampQueryManager.ts
@@ -4,9 +4,6 @@ export default class TimestampQueryManager {
   // class does nothing.
   timestampSupported: boolean;
 
-  // Number of timestamp counters
-  timestampCount: number;
-
   // The query objects. This is meant to be used in a ComputePassDescriptor's
   // or RenderPassDescriptor's 'timestampWrites' field.
   timestampQuerySet: GPUQuerySet;
@@ -17,37 +14,51 @@ export default class TimestampQueryManager {
   // A buffer to map this result back to CPU
   timestampMapBuffer: GPUBuffer;
 
-  // State used to avoid firing concurrent readback of timestamp values
-  hasOngoingTimestampReadback: boolean;
+  // Last times
+  timestamps: number[];
 
   // Device must have the "timestamp-query" feature
-  constructor(device: GPUDevice, timestampCount: number) {
+  constructor(device: GPUDevice, timestampPairCount: number) {
     this.timestampSupported = device.features.has('timestamp-query');
     if (!this.timestampSupported) return;
 
-    this.timestampCount = timestampCount;
+    this.timestamps = Array(timestampPairCount).fill(0);
 
     // Create timestamp queries
     this.timestampQuerySet = device.createQuerySet({
       type: 'timestamp',
-      count: timestampCount, // begin and end
+      count: timestampPairCount * 2, // begin and end
     });
 
     // Create a buffer where to store the result of GPU queries
     const timestampByteSize = 8; // timestamps are uint64
-    const timestampBufferSize = timestampCount * timestampByteSize;
     this.timestampBuffer = device.createBuffer({
-      size: timestampBufferSize,
+      size: this.timestampQuerySet.count * timestampByteSize,
       usage: GPUBufferUsage.COPY_SRC | GPUBufferUsage.QUERY_RESOLVE,
     });
 
     // Create a buffer to map the result back to the CPU
     this.timestampMapBuffer = device.createBuffer({
-      size: timestampBufferSize,
+      size: this.timestampBuffer.size,
       usage: GPUBufferUsage.COPY_DST | GPUBufferUsage.MAP_READ,
     });
+  }
 
-    this.hasOngoingTimestampReadback = false;
+  // Add both a start and end timestamp.
+  addTimestampWrite(
+    renderPassDescriptor: GPURenderPassDescriptor,
+    pairId: number
+  ) {
+    if (this.timestampSupported) {
+      // We instruct the render pass to write to the timestamp query before/after
+      const ndx = pairId * 2;
+      renderPassDescriptor.timestampWrites = {
+        querySet: this.timestampQuerySet,
+        beginningOfPassWriteIndex: ndx,
+        endOfPassWriteIndex: ndx + 1,
+      };
+    }
+    return renderPassDescriptor;
   }
 
   // Resolve all timestamp queries and copy the result into the map buffer
@@ -59,13 +70,13 @@ export default class TimestampQueryManager {
     commandEncoder.resolveQuerySet(
       this.timestampQuerySet,
       0 /* firstQuery */,
-      this.timestampCount /* queryCount */,
+      this.timestampQuerySet.count /* queryCount */,
       this.timestampBuffer,
       0 /* destinationOffset */
     );
 
-    if (!this.hasOngoingTimestampReadback) {
-      // Copy values to the mapped buffer
+    if (this.timestampMapBuffer.mapState === 'unmapped') {
+      // Copy values to the mappable buffer
       commandEncoder.copyBufferToBuffer(
         this.timestampBuffer,
         0,
@@ -76,22 +87,27 @@ export default class TimestampQueryManager {
     }
   }
 
-  // Once resolved, we can read back the value of timestamps
-  readAsync(onTimestampReadBack: (timestamps: BigUint64Array) => void): void {
+  // Read the value of timestamps.
+  update(): void {
     if (!this.timestampSupported) return;
-    if (this.hasOngoingTimestampReadback) return;
-
-    this.hasOngoingTimestampReadback = true;
+    if (this.timestampMapBuffer.mapState !== 'unmapped') return;
 
     const buffer = this.timestampMapBuffer;
     void buffer.mapAsync(GPUMapMode.READ).then(() => {
       const rawData = buffer.getMappedRange();
       const timestamps = new BigUint64Array(rawData);
-
-      onTimestampReadBack(timestamps);
-
+      for (let i = 0; i < this.timestamps.length; ++i) {
+        const ndx = i * 2;
+        // Cast into number. Number can be 9007199254740991 as max integer
+        // which is 109 days of nano seconds.
+        const elapsedNs = Number(timestamps[ndx + 1] - timestamps[ndx]);
+        // It's possible elapsedNs is negative which means it's invalid
+        // (see spec https://gpuweb.github.io/gpuweb/#timestamp)
+        if (elapsedNs >= 0) {
+          this.timestamps[i] = elapsedNs;
+        }
+      }
       buffer.unmap();
-      this.hasOngoingTimestampReadback = false;
     });
   }
 }
diff --git a/sample/timestampQuery/main.ts b/sample/timestampQuery/main.ts
index 66f11ad0..326619b7 100644
--- a/sample/timestampQuery/main.ts
+++ b/sample/timestampQuery/main.ts
@@ -172,14 +172,10 @@ const renderPassDescriptor: GPURenderPassDescriptor = {
     depthLoadOp: 'clear',
     depthStoreOp: 'store',
   },
-  // We instruct the render pass to write to the timestamp query before/after
-  timestampWrites: {
-    querySet: timestampQueryManager.timestampQuerySet,
-    beginningOfPassWriteIndex: 0,
-    endOfPassWriteIndex: 1,
-  },
 };
 
+timestampQueryManager.addTimestampWrite(renderPassDescriptor, 0);
+
 const aspect = canvas.width / canvas.height;
 const projectionMatrix = mat4.perspective((2 * Math.PI) / 5, aspect, 1, 100.0);
 const modelViewProjectionMatrix = mat4.create();
@@ -222,32 +218,19 @@ function frame() {
   passEncoder.end();
 
   // Resolve timestamp queries, so that their result is available in
-  // a GPU-sude buffer.
+  // a GPU-side buffer.
   timestampQueryManager.resolveAll(commandEncoder);
 
   device.queue.submit([commandEncoder.finish()]);
 
-  // Read timestamp value back from GPU buffers
-  timestampQueryManager.readAsync((timestamps) => {
-    // This may happen (see spec https://gpuweb.github.io/gpuweb/#timestamp)
-    if (timestamps[1] < timestamps[0]) return;
-
-    // Measure difference (in bigints)
-    const elapsedNs = timestamps[1] - timestamps[0];
-    // Cast into regular int (ok because value is small after difference)
-    // and convert from nanoseconds to milliseconds:
-    const elapsedMs = Number(elapsedNs) * 1e-6;
-    renderPassDurationCounter.addSample(elapsedMs);
-    console.log(
-      'timestamps (ms): elapsed',
-      elapsedMs,
-      'avg',
-      renderPassDurationCounter.getAverage()
-    );
-    perfDisplay.innerHTML = `Render Pass duration: ${renderPassDurationCounter
-      .getAverage()
-      .toFixed(3)} ms ± ${renderPassDurationCounter.getStddev().toFixed(3)} ms`;
-  });
+  timestampQueryManager.update();
+  const elapsedNs = timestampQueryManager.timestamps[0];
+  // Convert from nanoseconds to milliseconds:
+  const elapsedMs = Number(elapsedNs) * 1e-6;
+  renderPassDurationCounter.addSample(elapsedMs);
+  perfDisplay.innerHTML = `Render Pass duration: ${renderPassDurationCounter
+    .getAverage()
+    .toFixed(3)} ms ± ${renderPassDurationCounter.getStddev().toFixed(3)} ms`;
 
   requestAnimationFrame(frame);
 }

From 469f3abb7082838765d5d33af3216c401cd89a26 Mon Sep 17 00:00:00 2001
From: Gregg Tavares <github@greggman.com>
Date: Mon, 18 Nov 2024 18:26:58 -0800
Subject: [PATCH 2/3] address comments

---
 .../timestampQuery/TimestampQueryManager.ts   | 84 +++++++++----------
 sample/timestampQuery/index.html              | 13 +++
 sample/timestampQuery/main.ts                 | 42 ++++------
 3 files changed, 69 insertions(+), 70 deletions(-)

diff --git a/sample/timestampQuery/TimestampQueryManager.ts b/sample/timestampQuery/TimestampQueryManager.ts
index 54be3c42..1bdb7fc4 100644
--- a/sample/timestampQuery/TimestampQueryManager.ts
+++ b/sample/timestampQuery/TimestampQueryManager.ts
@@ -6,106 +6,102 @@ export default class TimestampQueryManager {
 
   // The query objects. This is meant to be used in a ComputePassDescriptor's
   // or RenderPassDescriptor's 'timestampWrites' field.
-  timestampQuerySet: GPUQuerySet;
+  #timestampQuerySet: GPUQuerySet;
 
   // A buffer where to store query results
-  timestampBuffer: GPUBuffer;
+  #timestampBuffer: GPUBuffer;
 
   // A buffer to map this result back to CPU
-  timestampMapBuffer: GPUBuffer;
+  #timestampMapBuffer: GPUBuffer;
 
-  // Last times
-  timestamps: number[];
+  // Last queried elapsed time of the pass.
+  passElapsedTime: number;
 
   // Device must have the "timestamp-query" feature
-  constructor(device: GPUDevice, timestampPairCount: number) {
+  constructor(device: GPUDevice) {
     this.timestampSupported = device.features.has('timestamp-query');
     if (!this.timestampSupported) return;
 
-    this.timestamps = Array(timestampPairCount).fill(0);
+    this.passElapsedTime = 0;
 
     // Create timestamp queries
-    this.timestampQuerySet = device.createQuerySet({
+    this.#timestampQuerySet = device.createQuerySet({
       type: 'timestamp',
-      count: timestampPairCount * 2, // begin and end
+      count: 2, // begin and end
     });
 
     // Create a buffer where to store the result of GPU queries
     const timestampByteSize = 8; // timestamps are uint64
-    this.timestampBuffer = device.createBuffer({
-      size: this.timestampQuerySet.count * timestampByteSize,
+    this.#timestampBuffer = device.createBuffer({
+      size: this.#timestampQuerySet.count * timestampByteSize,
       usage: GPUBufferUsage.COPY_SRC | GPUBufferUsage.QUERY_RESOLVE,
     });
 
     // Create a buffer to map the result back to the CPU
-    this.timestampMapBuffer = device.createBuffer({
-      size: this.timestampBuffer.size,
+    this.#timestampMapBuffer = device.createBuffer({
+      size: this.#timestampBuffer.size,
       usage: GPUBufferUsage.COPY_DST | GPUBufferUsage.MAP_READ,
     });
   }
 
   // Add both a start and end timestamp.
   addTimestampWrite(
-    renderPassDescriptor: GPURenderPassDescriptor,
-    pairId: number
+    passDescriptor: GPURenderPassDescriptor | GPUComputePassDescriptor
   ) {
     if (this.timestampSupported) {
       // We instruct the render pass to write to the timestamp query before/after
-      const ndx = pairId * 2;
-      renderPassDescriptor.timestampWrites = {
-        querySet: this.timestampQuerySet,
-        beginningOfPassWriteIndex: ndx,
-        endOfPassWriteIndex: ndx + 1,
+      passDescriptor.timestampWrites = {
+        querySet: this.#timestampQuerySet,
+        beginningOfPassWriteIndex: 0,
+        endOfPassWriteIndex: 1,
       };
     }
-    return renderPassDescriptor;
+    return passDescriptor;
   }
 
-  // Resolve all timestamp queries and copy the result into the map buffer
-  resolveAll(commandEncoder: GPUCommandEncoder) {
+  // Resolve the timestamp queries and copy the result into the mappable buffer if possible.
+  resolve(commandEncoder: GPUCommandEncoder) {
     if (!this.timestampSupported) return;
 
     // After the end of the measured render pass, we resolve queries into a
     // dedicated buffer.
     commandEncoder.resolveQuerySet(
-      this.timestampQuerySet,
+      this.#timestampQuerySet,
       0 /* firstQuery */,
-      this.timestampQuerySet.count /* queryCount */,
-      this.timestampBuffer,
+      this.#timestampQuerySet.count /* queryCount */,
+      this.#timestampBuffer,
       0 /* destinationOffset */
     );
 
-    if (this.timestampMapBuffer.mapState === 'unmapped') {
+    if (this.#timestampMapBuffer.mapState === 'unmapped') {
       // Copy values to the mappable buffer
       commandEncoder.copyBufferToBuffer(
-        this.timestampBuffer,
+        this.#timestampBuffer,
         0,
-        this.timestampMapBuffer,
+        this.#timestampMapBuffer,
         0,
-        this.timestampBuffer.size
+        this.#timestampBuffer.size
       );
     }
   }
 
-  // Read the value of timestamps.
-  update(): void {
+  // Read the values of timestamps.
+  tryInitiateTimestampDownload(): void {
     if (!this.timestampSupported) return;
-    if (this.timestampMapBuffer.mapState !== 'unmapped') return;
+    if (this.#timestampMapBuffer.mapState !== 'unmapped') return;
 
-    const buffer = this.timestampMapBuffer;
+    const buffer = this.#timestampMapBuffer;
     void buffer.mapAsync(GPUMapMode.READ).then(() => {
       const rawData = buffer.getMappedRange();
       const timestamps = new BigUint64Array(rawData);
-      for (let i = 0; i < this.timestamps.length; ++i) {
-        const ndx = i * 2;
-        // Cast into number. Number can be 9007199254740991 as max integer
-        // which is 109 days of nano seconds.
-        const elapsedNs = Number(timestamps[ndx + 1] - timestamps[ndx]);
-        // It's possible elapsedNs is negative which means it's invalid
-        // (see spec https://gpuweb.github.io/gpuweb/#timestamp)
-        if (elapsedNs >= 0) {
-          this.timestamps[i] = elapsedNs;
-        }
+      // Subtract the begin time from the end time.
+      // Cast into number. Number can be 9007199254740991 as max integer
+      // which is 109 days of nano seconds.
+      const elapsedNs = Number(timestamps[1] - timestamps[0]);
+      // It's possible elapsedNs is negative which means it's invalid
+      // (see spec https://gpuweb.github.io/gpuweb/#timestamp)
+      if (elapsedNs >= 0) {
+        this.passElapsedTime = elapsedNs;
       }
       buffer.unmap();
     });
diff --git a/sample/timestampQuery/index.html b/sample/timestampQuery/index.html
index 5094605f..a25ccfbb 100644
--- a/sample/timestampQuery/index.html
+++ b/sample/timestampQuery/index.html
@@ -20,11 +20,24 @@
         max-width: 100%;
         display: block;
       }
+      #info {
+        color: white;
+        background-color: black;
+        position: absolute;
+        top: 10px;
+        left: 10px;
+      }
+      #info pre {
+        margin: 0.5em;
+      }
     </style>
     <script defer src="main.js" type="module"></script>
     <script defer type="module" src="../../js/iframe-helper.js"></script>
   </head>
   <body>
     <canvas></canvas>
+    <div id="info">
+      <pre></pre>
+    </div>
   </body>
 </html>
diff --git a/sample/timestampQuery/main.ts b/sample/timestampQuery/main.ts
index 326619b7..10dedac3 100644
--- a/sample/timestampQuery/main.ts
+++ b/sample/timestampQuery/main.ts
@@ -33,7 +33,7 @@ quitIfWebGPUNotAvailable(adapter, device);
 // NB: Look for 'timestampQueryManager' in this file to locate parts of this
 // snippets that are related to timestamps. Most of the logic is in
 // TimestampQueryManager.ts.
-const timestampQueryManager = new TimestampQueryManager(device, 2);
+const timestampQueryManager = new TimestampQueryManager(device);
 const renderPassDurationCounter = new PerfCounter();
 
 const context = canvas.getContext('webgpu') as GPUCanvasContext;
@@ -48,21 +48,7 @@ context.configure({
   format: presentationFormat,
 });
 
-// UI for perf counter
-const perfDisplayContainer = document.createElement('div');
-perfDisplayContainer.style.color = 'white';
-perfDisplayContainer.style.background = 'black';
-perfDisplayContainer.style.position = 'absolute';
-perfDisplayContainer.style.top = '10px';
-perfDisplayContainer.style.left = '10px';
-
-const perfDisplay = document.createElement('pre');
-perfDisplayContainer.appendChild(perfDisplay);
-if (canvas.parentNode) {
-  canvas.parentNode.appendChild(perfDisplayContainer);
-} else {
-  console.error('canvas.parentNode is null');
-}
+const perfDisplay = document.querySelector('#info pre');
 
 if (!supportsTimestampQueries) {
   perfDisplay.innerHTML = 'Timestamp queries are not supported';
@@ -174,7 +160,7 @@ const renderPassDescriptor: GPURenderPassDescriptor = {
   },
 };
 
-timestampQueryManager.addTimestampWrite(renderPassDescriptor, 0);
+timestampQueryManager.addTimestampWrite(renderPassDescriptor);
 
 const aspect = canvas.width / canvas.height;
 const projectionMatrix = mat4.perspective((2 * Math.PI) / 5, aspect, 1, 100.0);
@@ -219,18 +205,22 @@ function frame() {
 
   // Resolve timestamp queries, so that their result is available in
   // a GPU-side buffer.
-  timestampQueryManager.resolveAll(commandEncoder);
+  timestampQueryManager.resolve(commandEncoder);
 
   device.queue.submit([commandEncoder.finish()]);
 
-  timestampQueryManager.update();
-  const elapsedNs = timestampQueryManager.timestamps[0];
-  // Convert from nanoseconds to milliseconds:
-  const elapsedMs = Number(elapsedNs) * 1e-6;
-  renderPassDurationCounter.addSample(elapsedMs);
-  perfDisplay.innerHTML = `Render Pass duration: ${renderPassDurationCounter
-    .getAverage()
-    .toFixed(3)} ms ± ${renderPassDurationCounter.getStddev().toFixed(3)} ms`;
+  if (timestampQueryManager.timestampSupported) {
+    // Show the last successfully downloaded elapsed time.
+    const elapsedNs = timestampQueryManager.passElapsedTime;
+    // Convert from nanoseconds to milliseconds:
+    const elapsedMs = Number(elapsedNs) * 1e-6;
+    renderPassDurationCounter.addSample(elapsedMs);
+    perfDisplay.innerHTML = `Render Pass duration: ${renderPassDurationCounter
+      .getAverage()
+      .toFixed(3)} ms ± ${renderPassDurationCounter.getStddev().toFixed(3)} ms`;
+  }
+
+  timestampQueryManager.tryInitiateTimestampDownload();
 
   requestAnimationFrame(frame);
 }

From ff0c3e7008c6eaed0dd895cec7e9822207247440 Mon Sep 17 00:00:00 2001
From: Gregg Tavares <github@greggman.com>
Date: Mon, 18 Nov 2024 19:02:11 -0800
Subject: [PATCH 3/3] address comments

---
 sample/timestampQuery/TimestampQueryManager.ts | 8 ++++----
 sample/timestampQuery/main.ts                  | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/sample/timestampQuery/TimestampQueryManager.ts b/sample/timestampQuery/TimestampQueryManager.ts
index 1bdb7fc4..4d0533d3 100644
--- a/sample/timestampQuery/TimestampQueryManager.ts
+++ b/sample/timestampQuery/TimestampQueryManager.ts
@@ -14,15 +14,15 @@ export default class TimestampQueryManager {
   // A buffer to map this result back to CPU
   #timestampMapBuffer: GPUBuffer;
 
-  // Last queried elapsed time of the pass.
-  passElapsedTime: number;
+  // Last queried elapsed time of the pass in nanoseconds.
+  passDurationMeasurementNs: number;
 
   // Device must have the "timestamp-query" feature
   constructor(device: GPUDevice) {
     this.timestampSupported = device.features.has('timestamp-query');
     if (!this.timestampSupported) return;
 
-    this.passElapsedTime = 0;
+    this.passDurationMeasurementNs = 0;
 
     // Create timestamp queries
     this.#timestampQuerySet = device.createQuerySet({
@@ -101,7 +101,7 @@ export default class TimestampQueryManager {
       // It's possible elapsedNs is negative which means it's invalid
       // (see spec https://gpuweb.github.io/gpuweb/#timestamp)
       if (elapsedNs >= 0) {
-        this.passElapsedTime = elapsedNs;
+        this.passDurationMeasurementNs = elapsedNs;
       }
       buffer.unmap();
     });
diff --git a/sample/timestampQuery/main.ts b/sample/timestampQuery/main.ts
index 10dedac3..374966c0 100644
--- a/sample/timestampQuery/main.ts
+++ b/sample/timestampQuery/main.ts
@@ -211,7 +211,7 @@ function frame() {
 
   if (timestampQueryManager.timestampSupported) {
     // Show the last successfully downloaded elapsed time.
-    const elapsedNs = timestampQueryManager.passElapsedTime;
+    const elapsedNs = timestampQueryManager.passDurationMeasurementNs;
     // Convert from nanoseconds to milliseconds:
     const elapsedMs = Number(elapsedNs) * 1e-6;
     renderPassDurationCounter.addSample(elapsedMs);