1252971 - Memory panel docs update

Status: RESOLVED FIXED

(DevTools :: Memory, defect, P1)

Description

[Greg Tatum [:gregtatum]]

After going through a deep dive into the memory tool I've updated
and clarified a few things in the docs that I found confusing.

Comment 1

[Greg Tatum [:gregtatum]]

Attachment: Memory panel docs update

After going through a deep dive into the memory tool I've updated
and clarified a few things in the docs that I found confusing.

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8725806 [details] [diff] [review]
Memory panel docs update

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

Thanks! r=me with comments below addressed

::: devtools/docs/memory-panel.md
@@ +1,5 @@
>  # Memory Tool Architecture
>  
>  The memory tool is built of three main elements:
>  
> +1. The live heap graph exists as highly optimized C++ code. In order to get a

This first sentence rubs me the wrong way. Abstractly, the heap graph is the result of considering objects/strings/etc in memory as vertices in a graph and the references from one to another as a directed edge in that graph. Concretely, this is implemented in C++ by the allocator and garbage collector.

@@ +2,5 @@
>  
>  The memory tool is built of three main elements:
>  
> +1. The live heap graph exists as highly optimized C++ code. In order to get a
> +   clear picture of what is going in the heap graph a specialized interface is

"... what is going in the heap graph <comma> a specialized interface ..."

@@ +7,5 @@
> +   created to represent its state. The `JS::ubi::Node` is the basis for this
> +   representation. This interface can be created from the live heap graph, or a
> +   serialized, offline snapshot from a previous moment in time. Our various heap
> +   analyses (census, dominator trees, shortest paths, etc) run on top of
> +   `JS::ubi::Node` graphs. The `ubi` in the name stands for ubiquitous and is a

Put "ubiquitous" in quotes.

@@ +8,5 @@
> +   representation. This interface can be created from the live heap graph, or a
> +   serialized, offline snapshot from a previous moment in time. Our various heap
> +   analyses (census, dominator trees, shortest paths, etc) run on top of
> +   `JS::ubi::Node` graphs. The `ubi` in the name stands for ubiquitous and is a
> +   namespace for all of the memory analysis C++ code.

"... and provides a namespace for memory analyses in C++ code."

@@ +39,5 @@
>  
>  A "heap snapshot" is a representation of the heap graph at some particular past
>  instance in time.
>  
> +A "heap analysis" is any number of algorithms that runs on a `JS::ubi::Node`

"Analysis" is singular, while "algorithms" is plural, and "runs" works with the singular form, not the plural form. The change to this sentence should be reverted, or further changed to:

  "Heap analyses" are algorithms that run on a `JS::ubi::Node` heap graph.

Personally, I prefer the existing sentence.

@@ +203,5 @@
> +state machine describing the snapshot states. Any of these states may go to the
> +ERROR state, from which they can never leave.
> +
> +```
> +SAVING -> SAVED -> READING -> READ     SAVED_CENSUS

Might as well take a moment to use "→" instead of "->" here, since we are already using unicode for the other arrows.

Comment 3

[Greg Tatum [:gregtatum]]

Attachment: Memory panel docs update -r=fitzgen

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7339ce7f9e5d

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7339ce7f9e5d