Make binary profile format parsing use Uint8Array instead of ArrayBuffer (#5678)

Accepting an ArrayBuffer is a bit restrictive because it means the
buffer has to be sized to exactly contain the profile bytes and nothing
else. Accept a Uint8Array is more flexible because the Uint8Array can
refer to just the sub-part of the buffer that actually contains the
profile.

This lets us avoid a copy in symbolicator-cli.

Author: mstange

src/profile-logic/import/art-trace.ts

@@ -194,12 +194,12 @@ type ArtTrace = {
 };
 
 function detectArtTraceFormat(
-  traceBuffer: ArrayBufferLike
+  traceBuffer: Uint8Array
 ): 'regular' | 'streaming' | 'unrecognized' {
   try {
     const lengthOfExpectedFirstTwoLinesOfSummarySection = '*version\nX\n'
       .length;
-    const firstTwoLinesBuffer = traceBuffer.slice(
+    const firstTwoLinesBuffer = traceBuffer.subarray(
       0,
       lengthOfExpectedFirstTwoLinesOfSummarySection
     );
@@ -213,7 +213,11 @@ function detectArtTraceFormat(
   }
 
   try {
-    const dataView = new DataView(traceBuffer);
+    const dataView = new DataView(
+      traceBuffer.buffer,
+      traceBuffer.byteOffset,
+      traceBuffer.byteLength
+    );
     const magic = dataView.getUint32(0, true);
     if (magic === TRACE_MAGIC) {
       return 'streaming';
@@ -523,9 +527,9 @@ function parseStreamingFormat(reader: ByteReader) {
   };
 }
 
-function parseArtTrace(buffer: ArrayBufferLike): ArtTrace {
+function parseArtTrace(buffer: Uint8Array): ArtTrace {
   try {
-    const reader = new ByteReader(new Uint8Array(buffer));
+    const reader = new ByteReader(buffer);
     switch (detectArtTraceFormat(buffer)) {
       case 'regular':
         return parseRegularFormat(reader);
@@ -915,13 +919,13 @@ class ThreadBuilder {
   }
 }
 
-export function isArtTraceFormat(traceBuffer: ArrayBufferLike) {
+export function isArtTraceFormat(traceBuffer: Uint8Array) {
   return detectArtTraceFormat(traceBuffer) !== 'unrecognized';
 }
 
 // Convert an ART trace to the Gecko profile format.
 export function convertArtTraceProfile(
-  traceBuffer: ArrayBufferLike
+  traceBuffer: Uint8Array
 ): GeckoProfileVersion11 {
   const trace = parseArtTrace(traceBuffer);
   const originalIntervalInUsec = procureSamplingInterval(trace);

src/profile-logic/import/simpleperf.ts

@@ -474,13 +474,17 @@ class FirefoxProfile {
 }
 
 export class SimpleperfReportConverter {
-  buffer: ArrayBufferLike;
+  buffer: Uint8Array;
   bufferView: DataView;
   bufferOffset: number = 0;
 
-  constructor(buffer: ArrayBufferLike) {
+  constructor(buffer: Uint8Array) {
     this.buffer = buffer;
-    this.bufferView = new DataView(buffer);
+    this.bufferView = new DataView(
+      buffer.buffer,
+      buffer.byteOffset,
+      buffer.byteLength
+    );
   }
 
   readUint16LE() {
@@ -509,11 +513,10 @@ export class SimpleperfReportConverter {
   }
 
   readRecord(recordSize: number): report.Record {
-    const recordBuffer = this.buffer.slice(
+    const recordArray = this.buffer.subarray(
       this.bufferOffset,
       this.bufferOffset + recordSize
     );
-    const recordArray = new Uint8Array(recordBuffer);
     this.bufferOffset += recordSize;
 
     return report.Record.decode(recordArray);
@@ -577,7 +580,7 @@ export class SimpleperfReportConverter {
 }
 
 export function convertSimpleperfTraceProfile(
-  traceBuffer: ArrayBufferLike
+  traceBuffer: Uint8Array
 ): Profile {
   return new SimpleperfReportConverter(traceBuffer).process();
 }

src/profile-logic/process-profile.ts

@@ -1965,27 +1965,33 @@ export async function unserializeProfileOfArbitraryFormat(
     // object is constructed from an ArrayBuffer in a different context... which
     // happens in our tests.
     if (String(arbitraryFormat) === '[object ArrayBuffer]') {
-      let arrayBuffer = arbitraryFormat as ArrayBuffer;
+      const arrayBuffer = arbitraryFormat as ArrayBuffer;
+      arbitraryFormat = new Uint8Array(arrayBuffer);
+    }
 
+    // Handle binary formats.
+    if (
+      arbitraryFormat instanceof Uint8Array ||
+      (globalThis.Buffer && arbitraryFormat instanceof globalThis.Buffer)
+    ) {
       // Check for the gzip magic number in the header. If we find it, decompress
       // the data first.
-      const profileBytes = new Uint8Array(arrayBuffer);
+      let profileBytes = arbitraryFormat as Uint8Array<ArrayBuffer>;
       if (isGzip(profileBytes)) {
-        const decompressedProfile = await decompress(profileBytes);
-        arrayBuffer = decompressedProfile.buffer;
+        profileBytes = await decompress(profileBytes);
       }
 
-      if (isArtTraceFormat(arrayBuffer)) {
-        arbitraryFormat = convertArtTraceProfile(arrayBuffer);
-      } else if (verifyMagic(SIMPLEPERF_MAGIC, arrayBuffer)) {
+      if (isArtTraceFormat(profileBytes)) {
+        arbitraryFormat = convertArtTraceProfile(profileBytes);
+      } else if (verifyMagic(SIMPLEPERF_MAGIC, profileBytes)) {
         const { convertSimpleperfTraceProfile } = await import(
           './import/simpleperf'
         );
-        arbitraryFormat = convertSimpleperfTraceProfile(arrayBuffer);
+        arbitraryFormat = convertSimpleperfTraceProfile(profileBytes);
       } else {
         try {
           const textDecoder = new TextDecoder(undefined, { fatal: true });
-          arbitraryFormat = await textDecoder.decode(arrayBuffer);
+          arbitraryFormat = await textDecoder.decode(profileBytes);
         } catch (e) {
           console.error('Source exception:', e);
           throw new Error(

src/symbolicator-cli/index.ts

@@ -85,15 +85,8 @@ export async function run(options: CliOptions) {
   // by our importers.
   const bytes = fs.readFileSync(options.input, null);
 
-  // bytes is a Uint8Array whose underlying ArrayBuffer can be longer than bytes.length.
-  // Copy the contents into a new ArrayBuffer which is sized correctly, so that we
-  // don't include uninitialized data from the extra parts of the underlying buffer.
-  // Alternatively, we could make unserializeProfileOfArbitraryFormat support
-  // Uint8Array or Buffer in addition to ArrayBuffer.
-  const byteBufferCopy = Uint8Array.prototype.slice.call(bytes).buffer;
-
   // Load the profile.
-  const profile = await unserializeProfileOfArbitraryFormat(byteBufferCopy);
+  const profile = await unserializeProfileOfArbitraryFormat(bytes);
   if (profile === undefined) {
     throw new Error('Unable to parse the profile.');
   }

src/test/unit/profile-conversion.test.ts

@@ -245,9 +245,8 @@ describe('converting Google Chrome profile', function () {
       'src/test/fixtures/upgrades/chrome-tracing.json.gz'
     );
     const decompressedBuffer = zlib.gunzipSync(compressedBuffer);
-    const profile = await unserializeProfileOfArbitraryFormat(
-      decompressedBuffer.buffer
-    );
+    const profile =
+      await unserializeProfileOfArbitraryFormat(decompressedBuffer);
     if (profile === undefined) {
       throw new Error('Unable to parse the profile.');
     }
@@ -263,9 +262,8 @@ describe('converting Google Chrome profile', function () {
       'src/test/fixtures/upgrades/chrome-trace-issue-5429.json.gz'
     );
     const decompressedBuffer = zlib.gunzipSync(compressedBuffer);
-    const profile = await unserializeProfileOfArbitraryFormat(
-      decompressedBuffer.buffer
-    );
+    const profile =
+      await unserializeProfileOfArbitraryFormat(decompressedBuffer);
     if (profile === undefined) {
       throw new Error('Unable to parse the profile.');
     }
@@ -425,8 +423,9 @@ describe('converting ART trace', function () {
     const buffer = fs.readFileSync(
       'src/test/fixtures/upgrades/art-trace-regular.trace.gz'
     );
-    const arrayBuffer = zlib.gunzipSync(buffer).buffer;
-    const profile = await unserializeProfileOfArbitraryFormat(arrayBuffer);
+    const uncompressedBytes = zlib.gunzipSync(buffer);
+    const profile =
+      await unserializeProfileOfArbitraryFormat(uncompressedBytes);
     if (profile === undefined) {
       throw new Error('Unable to parse the profile.');
     }
@@ -440,8 +439,9 @@ describe('converting ART trace', function () {
     const buffer = fs.readFileSync(
       'src/test/fixtures/upgrades/art-trace-streaming.trace.gz'
     );
-    const arrayBuffer = zlib.gunzipSync(buffer).buffer;
-    const profile = await unserializeProfileOfArbitraryFormat(arrayBuffer);
+    const uncompressedBytes = zlib.gunzipSync(buffer);
+    const profile =
+      await unserializeProfileOfArbitraryFormat(uncompressedBytes);
     if (profile === undefined) {
       throw new Error('Unable to parse the profile.');
     }
@@ -457,8 +457,9 @@ describe('converting Simpleperf trace', function () {
     const buffer = fs.readFileSync(
       'src/test/fixtures/upgrades/simpleperf-task-clock.trace.gz'
     );
-    const arrayBuffer = zlib.gunzipSync(buffer).buffer;
-    const profile = await unserializeProfileOfArbitraryFormat(arrayBuffer);
+    const uncompressedBytes = zlib.gunzipSync(buffer);
+    const profile =
+      await unserializeProfileOfArbitraryFormat(uncompressedBytes);
     if (profile === undefined) {
       throw new Error('Unable to parse the profile.');
     }
@@ -472,8 +473,9 @@ describe('converting Simpleperf trace', function () {
     const buffer = fs.readFileSync(
       'src/test/fixtures/upgrades/simpleperf-cpu-clock.trace.gz'
     );
-    const arrayBuffer = zlib.gunzipSync(buffer).buffer;
-    const profile = await unserializeProfileOfArbitraryFormat(arrayBuffer);
+    const uncompressedBytes = zlib.gunzipSync(buffer);
+    const profile =
+      await unserializeProfileOfArbitraryFormat(uncompressedBytes);
     if (profile === undefined) {
       throw new Error('Unable to parse the profile.');
     }

src/utils/magic.ts

@@ -1,9 +1,6 @@
 export const SIMPLEPERF = 'SIMPLEPERF';
 
-export function verifyMagic(
-  magic: string,
-  traceBuffer: ArrayBufferLike
-): boolean {
+export function verifyMagic(magic: string, traceBuffer: Uint8Array): boolean {
   return (
     new TextDecoder('utf8').decode(traceBuffer.slice(0, magic.length)) === magic
   );