1145442 - Documentation for JIT strategies

Status: RESOLVED FIXED

(Developer Documentation Graveyard :: Developer Tools, defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

If we use JIT strategy constant names as a unique identifier for optimization strategies, we can use this to link to additional information regarding what strategy failed. On the front end we can have the raw strategy attempts link to the docs, which could just be one or two simple sentences -- this is more of a reference, not suggestions on how to write fast code.

Not sure if we should link to MDN/wiki.moz/wherever there is JIT documentation, or maybe have it open up a new page `about:jitopts#GetProp_IC` or whatever. As this is mainly for JIT/SM engineers at the moment, and is documentation, I don't think this needs to be localized.

Comment 1

[Kannan Vijayan [:djvj]]

I think MDN is the best plan here.  That way, there's a public resource that's accessible outside of the tool as well.

How do I go about creating a new MDN page, and what path should I use for it?

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

What exactly will be documented? Optimizations? Data types? Would this be devtools specific, or apply to any use of our JITs? Wondering if it should live under js or devtools on mdn.

Pinging Will if there's any sort of format that should be used, although I suspect we'll need a slightly different parser than the CSS docs (will look more how we display those later today)

Comment 3

[Kannan Vijayan [:djvj]]

It think it's best if it lives under devtools for now on mdn.  If at some point there's a special 'jit' MDN category, it might fit under there.  But these are specific optimization strategy details relevant to a particular engine.  It relates to either engine details specifically, or devtools, not with "javascript" as a concept.

The first thing to document is optimizations.

So for example, somebody sees that a GetProp_InferredConstant failure showed up at an optimization site, they can go to a wiki page that tells them that this is an optimization that operates on "singleton" objects, and tries to infer that a value on this object is a constant on the basis that it hasn't been assigned to more than once.  They can learn how the optimization is defeated, and what kind of situations it's useful for.

Likewise for all the GetProp, SetProp, GetElem, and SetElem variants.

Comment 4

[Shu-yu Guo [:shu]]

What should the format of each entry be?

Kannan proposes 4 sections:

1. What it optimizes
2. How it tries to optimize
3. How can it fail
4. Common fixes

We should definitely write out in more detail for common opts (GetProp_DefiniteSlot). What of the rarely applicable ones, how do we compress the 4 sections above, as there are a myriad of ways they can fail.

Sections 1, 3, and 4 could all benefit greatly from concrete examples.

Comment 5

[Shu-yu Guo [:shu]]

A comprehensive "How it can fail" might grow very large very quickly.

I suppose the format might look more like this:

For each Strategy:

1. What it tries to optimize
2. How it can fail (list of links to Outcomes)
3. How it can succeed (list of links to Outcomes)

For each failure Outcome:

1. How it can fail
2. Common fixes

For each success Outcome:

1. Common patterns that succeed

Comment 6

[Kannan Vijayan [:djvj]]

That seems reasonable.  It does risk the potential of forcing the reader to go to different pages, which breaks their flow.  If they visit an optimization strategy, they should be able to get a pretty complete description of what they need right there.  Maybe not all the nitty gritty details, but a good overall picture of the optimization.

What if in the 'how it can fail' and 'how it can succeed' sections, in addition to a link to the list of outcomes (with detailed info), we present a blurb (small paragraph) summarizing the broad failure and success conditions?

Comment 7

[Kannan Vijayan [:djvj]]

Also, here's a copypaste of the list of current opt strategies. We should divvy these up:

Here's a copy-paste of the current set optimization strategies from OptimizationTrackingInfo.h.  We should just divvy this up somehow and knock 'em off one by one:

GetProp_ArgumentsLength
GetProp_ArgumentsCallee
GetProp_InferredConstant
GetProp_Constant
GetProp_StaticName
GetProp_SimdGetter
GetProp_TypedObject
GetProp_DefiniteSlot
GetProp_Unboxed
GetProp_CommonGetter
GetProp_InlineAccess
GetProp_Innerize
GetProp_InlineCache

SetProp_CommonSetter
SetProp_TypedObject
SetProp_DefiniteSlot
SetProp_Unboxed
SetProp_InlineAccess
SetProp_InlineCache

GetElem_TypedObject
GetElem_Dense
GetElem_TypedStatic
GetElem_TypedArray
GetElem_String
GetElem_Arguments
GetElem_ArgumentsInlined
GetElem_InlineCache

SetElem_TypedObject
SetElem_TypedStatic
SetElem_TypedArray
SetElem_Dense
SetElem_Arguments
SetElem_InlineCache

Call_Inline

Comment 8

[Shu-yu Guo [:shu]]

(In reply to Kannan Vijayan [:djvj] from comment #6)
> That seems reasonable.  It does risk the potential of forcing the reader to
> go to different pages, which breaks their flow.  If they visit an
> optimization strategy, they should be able to get a pretty complete
> description of what they need right there.  Maybe not all the nitty gritty
> details, but a good overall picture of the optimization.
> 
> What if in the 'how it can fail' and 'how it can succeed' sections, in
> addition to a link to the list of outcomes (with detailed info), we present
> a blurb (small paragraph) summarizing the broad failure and success
> conditions?

This sounds fine to me.

(In reply to Kannan Vijayan [:djvj] from comment #7)
> Also, here's a copypaste of the list of current opt strategies. We should
> divvy these up:
> 
> Here's a copy-paste of the current set optimization strategies from
> OptimizationTrackingInfo.h.  We should just divvy this up somehow and knock
> 'em off one by one:
> 

I just had one of them manager-type delegation ideas. WHAT IF, we held a documentation "party" with other JIT folks at Whistler and write some of this over drinks? Might be a good opportunity for each other to get a broader picture of the "macro" optimizations our JIT does.

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

Talked to Will about this, relaying here:

* The docs should live under Spidermonkey[0] docs somewhere, as these are all specific to Spidermonkey (and would be helpful on its own for a SM hacker), and nothing is "devtools" specific, as its just documenting SM's enums.

* The format doesn't matter for using tooltips in the tools -- only thing that matters is that they're consistent, as they get parsed for sections. For example, when parsing the CSS docs with the raw version[1] of MDN, only the summary and syntax sections are displayed. Each piece of documentation that is to be linked must also be anchor-linked, so we can just fetch a section by id[2].

I'm not sure what kind of docs should be listed here, w/r/t how it can be optimized, how it can fail/be fixed, etc., but whatever you think would be appropriate to anyone interested in SM. Some thoughts are we should probably only show a moderately short summary in the tooltip (with a link to MDN for more), and  while this is still geared towards JIT/SM hackers and still pretty inaccessible, maybe the "common fixes" docs can be the groundwork for a "JIT Coach" that can display tips in the call tree, or something.

Will offered to do editorial reviews and the like for any docs here, as well as any discussions in Whistler about this.

[0] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey
[1] https://developer.mozilla.org/en-US/docs/Web/CSS/background-color?raw&macros
[2] https://developer.mozilla.org/en-US/docs/Web/CSS/margin?raw&macros&section=Syntax

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

Looks like there'll be a docs jam later in the week at Whistler, if you want to overlap there, I'd imagine there'd be a few people from MDN to help out on any formatting and editorial questions.

via Jeremie Patonnier:
> From Basically, on Thursday and Friday the MDN team will take over the Algonqin room at the Fairmont
> Hotel and write documentation like the crazy foxes we are.

Comment 11

[Kannan Vijayan [:djvj]]

I wrote up a skeleton MDN page here.  Let me know what you guys think:

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JIT_Optimization_Strategies

Comment 12

[Jordan Santell [:jsantell] [@jsantell]]

Looks great so far -- is it possible to have one complete example of what info would be there? Can see how easy it'd be to link from the tools then

Comment 13

[Shu-yu Guo [:shu]]

Seconded. Looks good. I'll start writing some stuff about inlining.

Comment 14

[Shu-yu Guo [:shu]]

Attachment: Add JIT optimization tracking constant docs in tree. WIP

I started moving the docs in tree with some minor edits. Not done copying
everything yet.

Comment 15

[Kannan Vijayan [:djvj]]

Attachment: jitcoach-strategy-docs.patch

Reposing the markdown documentation patch.  Copied all the documentation from the wiki.  Filled in a couple more.  Let's check this in and work on it in pieces.

Comment 16

[Kannan Vijayan [:djvj]]

Attachment: jitcoach-strategy-docs.patch

Updated patch to include documentation for all getprops.

I'm questioning the need to add docs for the optimization outcomes directly to this document.  I think your earlier idea of keeping those in a separate place is a better idea.

It's a LOT of work to track down and document each of these on a per-optimization-strategy basis (and it's slowing down the strategy documentation, which is arguably more important than detailing the outcomes).

I think we should remove the outcomes entirely and move them to a different .md file.

Anyway, for this review just check over the explanations and check them for sanity and language.

Comment 17

[Shu-yu Guo [:shu]]

(In reply to Kannan Vijayan [:djvj] from comment #16)
> Created attachment 8640765 [details] [diff] [review]
> jitcoach-strategy-docs.patch
> 
> Updated patch to include documentation for all getprops.
> 
> I'm questioning the need to add docs for the optimization outcomes directly
> to this document.  I think your earlier idea of keeping those in a separate
> place is a better idea.
> 
> It's a LOT of work to track down and document each of these on a
> per-optimization-strategy basis (and it's slowing down the strategy
> documentation, which is arguably more important than detailing the outcomes).
> 
> I think we should remove the outcomes entirely and move them to a different
> .md file.

Sounds good to me. Put another way, the strategies documentation only documents the success outcome, and we move the failure outcomes to another file.

Comment 18

[Shu-yu Guo [:shu]]

Comment on attachment 8640765 [details] [diff] [review]
jitcoach-strategy-docs.patch

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

Thanks for copying stuff over.

::: js/src/doc/JITOptimizations/config.sh
@@ +1,3 @@
> +base-url https://developer.mozilla.org/en-US/docs/Tools/
> +
> +markdown Strategies.md JIT-Optimization-Strategies

Since we're moving outcomes to a separate file, this should probably be JIT-Optimization/Strategies instead of JIT-Optimization-Strategies

Comment 19

[Kannan Vijayan [:djvj]]

Attachment: jitcoach-strategy-docs.patch

Updated strategy docs to include descriptions for all optimization strategies.  Not the most full-fledged descriptions for all of them, but it gets _something_ in there which can be iterated on with time.

Comment 20

[Kannan Vijayan [:djvj]]

Attachment: jitcoach-strategy-docs.patch

Sorry for repeatedly doing this.  Last time I promise.

Split the strategies and outcomes into different .md files.  Fixed config file.  Removed outcome documentation from strategies file.

I'll push this after r+.  No more changes.  Promise.

Comment 21

[Shu-yu Guo [:shu]]

Comment on attachment 8641177 [details] [diff] [review]
jitcoach-strategy-docs.patch

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

Looks good, thanks for doing the copying.

::: js/src/doc/JITOptimizations/Outcomes.md
@@ +234,5 @@
> +more than one level.  The first level of recursion is inlineable.
> +
> +### CantInlineHeavyweight
> +
> +Unable to inline function call. The interpreted callee function contains

I had categorized the outcomes of inlining into 'unable' and 'unwilling', and the strategy page still refers to this distinction. Looks like you changed everything to 'unable', could you restore the 'unwilling' wording?

Comment 22

[Kannan Vijayan [:djvj]]

(In reply to Shu-yu Guo [:shu] from comment #21)
> Comment on attachment 8641177 [details] [diff] [review]
> I had categorized the outcomes of inlining into 'unable' and 'unwilling',
> and the strategy page still refers to this distinction. Looks like you
> changed everything to 'unable', could you restore the 'unwilling' wording?

I just checked the wiki.  The wording there uses 'unable' for everything.  Can you refer me to the unwilling wording?

Comment 23

[Kannan Vijayan [:djvj]]

Attachment: jitcoach-strategy-docs.patch

Trivial rebase.  Forwarding r+.

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b06bebbf0ed7

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b06bebbf0ed7

Comment 26

[Kannan Vijayan [:djvj]]

Attachment: update-jit-optimization-docs.patch

Patch fixing the markdown for the jitopt strategy docs, and adding a few more jitopt outcome docs.

Wiki pages have been updated with markdown generated output for both of these.

Comment 27

[Shu-yu Guo [:shu]]

Comment on attachment 8654366 [details] [diff] [review]
update-jit-optimization-docs.patch

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

::: js/src/doc/JITOptimizations/Strategies.md
@@ -77,5 @@
>  after it was first assigned, is likely a constant property.  It then directly
>  inlines the value of the property into hot code that accesses it. For
>  example, in the following code:
>  
> -```language-js

Why the removal of ```? Was it not being rendered properly?

Comment 28

[Kannan Vijayan [:djvj]]

(In reply to Shu-yu Guo [:shu] from comment #27)
> Why the removal of ```? Was it not being rendered properly?

Yeah, markdown proper didn't handle it, at least the one I pulled down with apt on my debian box.  Went for a more traditional syntax.

Comment 29

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/429b04491fe1

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/429b04491fe1

Comment 31

[Kannan Vijayan [:djvj]]

Attachment: jit-coach-optimization-docs.patch

More jit coach outcome documentation.  This completes nominal documentation for all the outcomes in the list.

Comment 32

[Shu-yu Guo [:shu]]

Comment on attachment 8658329 [details] [diff] [review]
jit-coach-optimization-docs.patch

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

Thanks!

::: js/src/doc/JITOptimizations/Outcomes.md
@@ +286,4 @@
>  ### ICGetPropStub_UnboxedRead
> +
> +An inline cache property read which retrieves an unboxed
> +value from an object.

I think this should be "retrieves a value from an unboxed object".

@@ +307,5 @@
> +
> +An inline cache property read which retrieves a
> +shadowed property from a DOM object.  A shadowed
> +property is an inbuilt DOM property that has been
> +overwritten by JS code.

nit: What's your character width set to? These seem real narrow.

Comment 33

[Kannan Vijayan [:djvj]]

(In reply to Shu-yu Guo [:shu] from comment #32)
> Comment on attachment 8658329 [details] [diff] [review]
> jit-coach-optimization-docs.patch
> nit: What's your character width set to? These seem real narrow.

100 chars.  I don't know why, I just ended up writing those with a narrow wordwrap.  Fixed.

Comment 34

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/435a86d7a398

Comment 35

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/435a86d7a398

Comment 36

[Kannan Vijayan [:djvj]]

Closing for now.  We can open a new bug if we need to add more docs.