831748 - refactor duplicated code

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Attachment: do it

The code for drawing raw stacks is mostly duplicated between the late writes an the hangs code. This patch fixes this. It also paves the was to adding fetch and hide links to the late write section by renaming the currently generic ones used by hangs.

Comment 1

[Nathan Froyd [:froydnj]]

Comment on attachment 703302 [details] [diff] [review]
do it

Review of attachment 703302 [details] [diff] [review]:
-----------------------------------------------------------------

Nice refactoring.

Comment 2

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/143e6064d88c

Comment 3

[Vladan Djeric (:vladan)]

Comment on attachment 703302 [details] [diff] [review]
do it

>+    function f() {}
>+    StackRenderer.renderStacks('late-writes', stacks, memoryMap, f);

This has landed already, but maybe in a follow up bug we can change this line to pass in "null" instead of a locally-defined function that does nothing

Comment 4

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

> >+    function f() {}
> >+    StackRenderer.renderStacks('late-writes', stacks, memoryMap, f);
> 
> This has landed already, but maybe in a follow up bug we can change this
> line to pass in "null" instead of a locally-defined function that does
> nothing

We could, but why would that be an improvement. Right now the callee can just call the callback without checking if it null or not.

IMHO a much better followup patch is to print a header in the late write stacks :-)

Comment 5

[Vladan Djeric (:vladan)]

(In reply to Rafael Ávila de Espíndola (:espindola) from comment #4)
> IMHO a much better followup patch is to print a header in the late write
> stacks :-)

That makes sense. My point was that calling back do-nothing locally-declared functions is hacky and is confusing to readers of the code. e.g. if I was reading renderStacks and saw the call to aRenderHeader(i), I'd be wondering what is the LateWrites header (there isn't one) and where it is defined

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/143e6064d88c