From bc966c455c4926e8d835c09a8f4240e049fce048 Mon Sep 17 00:00:00 2001 From: Kean Mariotti Date: Tue, 30 May 2023 20:11:06 +0000 Subject: [PATCH] Optimize bugreport loading Fix: b/269342535 Test: npm run test:all Change-Id: I89e03fff08fe1c7e7169a6eeb08efeda04d098d0 --- .../abt_chrome_extension_protocol.ts | 7 +- tools/winscope/src/app/mediator.ts | 4 +- tools/winscope/src/app/trace_pipeline.ts | 31 ++++++++ tools/winscope/src/app/trace_pipeline_test.ts | 68 ++++++++++++++++++ tools/winscope/src/common/file_utils.ts | 3 +- tools/winscope/src/common/time_utils.ts | 1 - tools/winscope/src/test/common/file_impl.ts | 6 +- tools/winscope/src/test/common/utils.ts | 9 ++- ...ta-UPB2.230407.019-2023-05-30-14-33-48.txt | 6 ++ .../bugreports/bugreport_stripped.zip | Bin 1035771 -> 1035794 bytes .../test/fixtures/bugreports/main_entry.txt | 1 + tools/winscope/src/trace/trace_file.ts | 4 +- 12 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 tools/winscope/src/test/fixtures/bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt create mode 100644 tools/winscope/src/test/fixtures/bugreports/main_entry.txt diff --git a/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts b/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts index a8aba832f..787cc2eab 100644 --- a/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts +++ b/tools/winscope/src/abt_chrome_extension/abt_chrome_extension_protocol.ts @@ -26,14 +26,15 @@ export class AbtChromeExtensionProtocol implements BuganizerAttachmentsDownloadE static readonly ABT_EXTENSION_ID = 'mbbaofdfoekifkfpgehgffcpagbbjkmj'; private onAttachmentsDownloadStart: OnBuganizerAttachmentsDownloadStart = FunctionUtils.DO_NOTHING; - private onttachmentsDownloaded: OnBuganizerAttachmentsDownloaded = FunctionUtils.DO_NOTHING_ASYNC; + private onAttachmentsDownloaded: OnBuganizerAttachmentsDownloaded = + FunctionUtils.DO_NOTHING_ASYNC; setOnBuganizerAttachmentsDownloadStart(callback: OnBuganizerAttachmentsDownloadStart) { this.onAttachmentsDownloadStart = callback; } setOnBuganizerAttachmentsDownloaded(callback: OnBuganizerAttachmentsDownloaded) { - this.onttachmentsDownloaded = callback; + this.onAttachmentsDownloaded = callback; } run() { @@ -83,7 +84,7 @@ export class AbtChromeExtensionProtocol implements BuganizerAttachmentsDownloadE }); const files = await Promise.all(filesBlobPromises); - await this.onttachmentsDownloaded(files); + await this.onAttachmentsDownloaded(files); } private isOpenBuganizerResponseMessage( diff --git a/tools/winscope/src/app/mediator.ts b/tools/winscope/src/app/mediator.ts index 3544a2cd6..60ee7eab8 100644 --- a/tools/winscope/src/app/mediator.ts +++ b/tools/winscope/src/app/mediator.ts @@ -83,7 +83,7 @@ export class Mediator { } ); - this.crossToolProtocol.setOnTimestampReceived(async (timestamp: Timestamp) => { + this.crossToolProtocol.setOnTimestampReceived((timestamp: Timestamp) => { this.onRemoteTimestampReceived(timestamp); }); @@ -203,7 +203,7 @@ export class Mediator { private async processRemoteFilesReceived(files: File[]) { this.resetAppToInitialState(); - this.processFiles(files); + await this.processFiles(files); } private async processFiles(files: File[]) { diff --git a/tools/winscope/src/app/trace_pipeline.ts b/tools/winscope/src/app/trace_pipeline.ts index 8ab16b70f..cad4c6120 100644 --- a/tools/winscope/src/app/trace_pipeline.ts +++ b/tools/winscope/src/app/trace_pipeline.ts @@ -37,6 +37,7 @@ class TracePipeline { traceFiles: TraceFile[], onLoadProgressUpdate: OnProgressUpdateType = FunctionUtils.DO_NOTHING ): Promise { + traceFiles = await this.filterBugreportFilesIfNeeded(traceFiles); const [parsers, parserErrors] = await this.parserFactory.createParsers( traceFiles, onLoadProgressUpdate @@ -111,6 +112,36 @@ class TracePipeline { this.files = new Map(); } + private async filterBugreportFilesIfNeeded(files: TraceFile[]): Promise { + const bugreportMainEntry = files.find((file) => file.file.name === 'main_entry.txt'); + if (!bugreportMainEntry) { + return files; + } + + const bugreportName = (await bugreportMainEntry.file.text()).trim(); + const isBugreport = files.find((file) => file.file.name === bugreportName) !== undefined; + if (!isBugreport) { + return files; + } + + const BUGREPORT_TRACE_DIRS = ['FS/data/misc/wmtrace/', 'FS/data/misc/perfetto-traces/']; + const isFileWithinBugreportTraceDir = (file: TraceFile) => { + for (const traceDir of BUGREPORT_TRACE_DIRS) { + if (file.file.name.startsWith(traceDir)) { + return true; + } + } + return false; + }; + + const fileBelongsToBugreport = (file: TraceFile) => + file.parentArchive === bugreportMainEntry.parentArchive; + + return files.filter((file) => { + return isFileWithinBugreportTraceDir(file) || !fileBelongsToBugreport(file); + }); + } + private getCommonTimestampType(): TimestampType { if (this.commonTimestampType !== undefined) { return this.commonTimestampType; diff --git a/tools/winscope/src/app/trace_pipeline_test.ts b/tools/winscope/src/app/trace_pipeline_test.ts index 20e01a07c..46c670177 100644 --- a/tools/winscope/src/app/trace_pipeline_test.ts +++ b/tools/winscope/src/app/trace_pipeline_test.ts @@ -39,6 +39,74 @@ describe('TracePipeline', () => { expect(traceEntries.get(TraceType.SURFACE_FLINGER)?.length).toBeGreaterThan(0); }); + it('can load bugreport and ignores non-trace dirs', async () => { + expect(tracePipeline.getLoadedTraces().length).toEqual(0); + + // Could be any file, we just need an instance of File to be used as a fake bugreport archive + const bugreportArchive = await UnitTestUtils.getFixtureFile( + 'bugreports/bugreport_stripped.zip' + ); + + const bugreportFiles = [ + new TraceFile( + await UnitTestUtils.getFixtureFile('bugreports/main_entry.txt', 'main_entry.txt'), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt', + 'bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt' + ), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/SurfaceFlinger.pb', + 'FS/data/misc/wmtrace/surface_flinger.bp' + ), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/Transactions.pb', + 'FS/data/misc/wmtrace/transactions.bp' + ), + bugreportArchive + ), + new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/WindowManager.pb', + 'FS/data/misc/ignored-dir/window_manager.bp' + ), + bugreportArchive + ), + ]; + + // Corner case: + // A plain trace file is loaded along the bugreport -> trace file must not be ignored + // + // Note: + // The even weirder corner case where two bugreports are loaded at the same time is + // currently not properly handled. + const plainTraceFile = new TraceFile( + await UnitTestUtils.getFixtureFile( + 'traces/elapsed_and_real_timestamp/InputMethodClients.pb', + 'would-be-ignored-if-was-part-of-bugreport/input_method_clients.pb' + ) + ); + + const mergedFiles = bugreportFiles.concat([plainTraceFile]); + const errors = await tracePipeline.loadTraceFiles(mergedFiles); + expect(errors.length).toEqual(0); + tracePipeline.buildTraces(); + const traces = tracePipeline.getTraces(); + + expect(traces.getTrace(TraceType.SURFACE_FLINGER)).toBeDefined(); + expect(traces.getTrace(TraceType.TRANSACTIONS)).toBeDefined(); + expect(traces.getTrace(TraceType.WINDOW_MANAGER)).toBeUndefined(); // ignored + expect(traces.getTrace(TraceType.INPUT_METHOD_CLIENTS)).toBeDefined(); + }); + it('is robust to invalid trace files', async () => { const invalidTraceFiles = [ new TraceFile(await UnitTestUtils.getFixtureFile('winscope_homepage.png')), diff --git a/tools/winscope/src/common/file_utils.ts b/tools/winscope/src/common/file_utils.ts index 447e043ad..ac2b688a7 100644 --- a/tools/winscope/src/common/file_utils.ts +++ b/tools/winscope/src/common/file_utils.ts @@ -61,9 +61,8 @@ class FileUtils { // Ignore directories continue; } else { - const name = FileUtils.removeDirFromFileName(filename); const fileBlob = await file.async('blob'); - const unzippedFile = new File([fileBlob], name); + const unzippedFile = new File([fileBlob], filename); unzippedFiles.push(unzippedFile); } diff --git a/tools/winscope/src/common/time_utils.ts b/tools/winscope/src/common/time_utils.ts index 876c3c1e8..42d6cf109 100644 --- a/tools/winscope/src/common/time_utils.ts +++ b/tools/winscope/src/common/time_utils.ts @@ -128,7 +128,6 @@ export class TimeUtils { } static formattedKotlinTimestamp(time: any, timestampType: TimestampType): string { - console.log('time', time); if (timestampType === TimestampType.ELAPSED) { return TimeUtils.format(new ElapsedTimestamp(BigInt(time.elapsedNanos.toString()))); } diff --git a/tools/winscope/src/test/common/file_impl.ts b/tools/winscope/src/test/common/file_impl.ts index 8ad842bcf..315bbe1b4 100644 --- a/tools/winscope/src/test/common/file_impl.ts +++ b/tools/winscope/src/test/common/file_impl.ts @@ -46,7 +46,11 @@ class FileImpl { } text(): Promise { - throw new Error('Not implemented!'); + const utf8Decoder = new TextDecoder(); + const text = utf8Decoder.decode(this.buffer); + return new Promise((resolve) => { + resolve(text); + }); } } diff --git a/tools/winscope/src/test/common/utils.ts b/tools/winscope/src/test/common/utils.ts index d7f1e8853..8c81a6394 100644 --- a/tools/winscope/src/test/common/utils.ts +++ b/tools/winscope/src/test/common/utils.ts @@ -18,9 +18,12 @@ import * as path from 'path'; import {FileImpl} from './file_impl'; class CommonTestUtils { - static async getFixtureFile(filename: string): Promise { - const buffer = CommonTestUtils.loadFixture(filename); - return new FileImpl(buffer, filename) as unknown as File; + static async getFixtureFile( + srcFilename: string, + dstFilename: string = srcFilename + ): Promise { + const buffer = CommonTestUtils.loadFixture(srcFilename); + return new FileImpl(buffer, dstFilename) as unknown as File; } static loadFixture(filename: string): ArrayBuffer { diff --git a/tools/winscope/src/test/fixtures/bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt b/tools/winscope/src/test/fixtures/bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt new file mode 100644 index 000000000..c02add504 --- /dev/null +++ b/tools/winscope/src/test/fixtures/bugreports/bugreport-codename_beta-UPB2.230407.019-2023-05-30-14-33-48.txt @@ -0,0 +1,6 @@ +======================================================== +== dumpstate: 2023-05-30 14:33:48 +======================================================== + +# ORIGINAL CONTENTS REMOVED TO AVOID INFORMATION LEAKS + diff --git a/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip b/tools/winscope/src/test/fixtures/bugreports/bugreport_stripped.zip index 36a2e7300ec5f7b3ec60a0ee44ebd6f0fc4f9bcc..c91a59532348846784ae4037148b755cbbdb0934 100644 GIT binary patch delta 1538 zcmbtUeN02%|3gNZBzGb*sEBr6mwGId*} zG16@+Alohz4aq<|#Zj`xRHw6Xe5|bP)IW?76FYo zch5cN-rqUr{xMke`kO`LTc~t|R-iRUoHfEbziDojAap#7M(fFs^+}wkmeQ1_GO{!j zv-zCCAAU=D2Fnbz>TbU6=4|O13ZW+$gbb0j#5#qGJ=J>#sAUe`zxC)Ezs<9%Qr#hs zI3U_-tR!>wN$LGhPhN_0g1F)C?qgO8Km!qtMn62#F$&njliUJ%HhH9-V3hk72=eQMsWWY+mDsPUPUHx!MOsZdA zaMpm9h$f8-Y^9x%DZ~*vU*X&D;0nKf2>wN|=ssj=Xl^l?TWsymX>EsXHIF4BrXs!b zpoosc$%v9obYUc`Hc_at4D>a;q&kSlE03OPG? zouJe6!Zuzn@Hx4>Ue71y=83))eZ^|>Y?u`&L{r4#>~nw}4`vGtqT%?CzK-N;BI>(7kT zZQ@j%>R0u^O?Bo*?u}Yi=Hq8;t$ocGdDySR-s}39p3&h>L8&D634FhI@e1BYf$F;_aT|O-n#3BqvyAK$ zI|ABALGDcAJeV*{!Gy0OK2ZwXC6Sxi8{zt^0tw{zDXa$`+DSt%PKbOx455=W)DNU; z8gmhs{gx?#yf^{AwO4T#efe6DU{er{nvw=q^WE_&SG9_rHIB#9grwD zP?Uju;m1d*tz_>EE`*S;%)kzgj1l(?HfD?c5wCj4p7>>>hwnWqS delta 1664 zcmaJ>ZA_b06h3cT`o1mgdl_GaeYk>dY>?RoXtxOjCrgF4z}SQ(iz{PsM$`?M5Sbe1 zjHVz{QqbKWE(*)yGP3|lV`>(4W^3sOU0_SlOmqhI2O2}rmZ*^p&u!@k8SyqJ={@H> z&$;LHIUP7(`R|97V?|t+00LyZ*3$?pr&lY1M`(jhkwZ7QiOy-~{;baD1^p+#=PgtN zu<$;MRF;XbWT3OpY?N&0g-I#-e1-#{5?C`2;D{8!7%lLwn<$-2)=3Tvbo};R%{c_J z@R|jFs9WEK1+FaY&+v5uyfmDjl=s%5(WY1jZ{nBINP)p^?yFmQ;p%q%{3&#@z*^J* zn@b1#&0FYu<3#VQ^_)z}o}(Z4IDmaD9zm&Lf$NV`r6!|8VjH> z+V2})VsP4n%NG#%(}DKsdlvv$I05WW>f^36C@GHvRaE99W%m1R9GV18iY7x#L6f8L zuKR7mWAhL5!Y>~jU*0v(10zUKzn!4ATWkuK#isnZUd;=E`9?!eJqI&GDUQcCP}7LA zoUHzB!R{DQqNSp#(9|x+h?uswh!?oFLzOqHWB_01CN(klYC=vp8qk84j`_(Rst=2@ z)4afZD38cVyzk}2`(O{oM8q9$X+6<=>_q3uQ;&9bcQ!ty27Z6m(P!?hSXXx{aI(#5 zDv%v$=D<=|t26KL=?ZVv*7c2!>HBt;^y+-RNz3$NmrCqV-?lidmLj9I*m&DetS|n? zpr1C#@~aM4Sq^8Ed^zw=I`+T|E#DrTVD-`1JcdJ}UcG7_^lnIOLdCN8KiEcMmt3Z> zXiCS#vP+WD<#hlzQMQSGlM+9YlP5(?oN?^<{%IVJLxqK5fC(0r=c%+5cE%7Bw=tNAgAwpNC^Uc9t0ZPQU5k~y0yVm?_ro%NDu@mgV1XlS#KD5c{z1s!hVvT3VL zQN|dQv_3%6=^Y;txgeeMMc`Zg2;VqLYy6QO&$4TMh>rS+$&@LL{zI}+Cn+kTY9<_Z zE}pp-68~4@w8Kv}veH_~(_z{XAo?xIQjDpu0JIUpDCtmGq;ny$I@Y;ng5*gxiz2NL zi}7(d