Sending an array of the same frames over and over for actions repeated multiple times in the same recording is wasteful. We should be de-duplicating both per-frame and per-stack.
Created attachment 354787 [details] Patch
Comment on attachment 354787 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354787&action=review > Source/WebInspectorUI/UserInterface/Models/Recording.js:322 > + case WI.Recording.Swizzle.CallStack: > + var array = await this.swizzle(data, WI.Recording.Swizzle.Array); > + var callFrames = []; > + for (let item of array) > + callFrames.push(await this.swizzle(item, WI.Recording.Swizzle.CallFrame)); > + this._swizzle[index][type] = callFrames; > + break; Why var instead of const/let? Also, what about this? const callFrames = array.map(item => { return await this.swizzle(item, WI.Recording.Swizzle.CallFrame); }); Hmmm... I guess that doesn't work because the arrow needs to be async. Can you do that?
Comment on attachment 354787 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354787&action=review >> Source/WebInspectorUI/UserInterface/Models/Recording.js:322 >> + break; > > Why var instead of const/let? > > Also, what about this? > > const callFrames = array.map(item => { return await this.swizzle(item, WI.Recording.Swizzle.CallFrame); }); > > Hmmm... I guess that doesn't work because the arrow needs to be async. Can you do that? `let` is block scoped, and a switch-case only has one block <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#Redeclarations> I'd have to do something like: this._swizzle[index][type] = await Promise.all(array.map((item) => this.swizzle(item, WI.Recording.Swizzle.CallFrame)); That reads a bit worse (more confusing) IMO.
Comment on attachment 354787 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=354787&action=review >>> Source/WebInspectorUI/UserInterface/Models/Recording.js:322 >>> + break; >> >> Why var instead of const/let? >> >> Also, what about this? >> >> const callFrames = array.map(item => { return await this.swizzle(item, WI.Recording.Swizzle.CallFrame); }); >> >> Hmmm... I guess that doesn't work because the arrow needs to be async. Can you do that? > > `let` is block scoped, and a switch-case only has one block > <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#Redeclarations> > > I'd have to do something like: > > this._swizzle[index][type] = await Promise.all(array.map((item) => this.swizzle(item, WI.Recording.Swizzle.CallFrame)); > > That reads a bit worse (more confusing) IMO. Actually, we should do this, as it will allow more things to be enqueued at once.
Created attachment 354841 [details] Patch
Comment on attachment 354841 [details] Patch Clearing flags on attachment: 354841 Committed r238199: <https://trac.webkit.org/changeset/238199>
All reviewed patches have been landed. Closing bug.
<rdar://problem/46074620>