-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add docs on how to think through inline directives #22
base: main
Are you sure you want to change the base?
Conversation
### Primitive Data Flow: Production and Consumption | ||
|
||
How does `purs-backend-es` know when unneeded intermediate data structures are being used and when it is safe to remove them? These usages arise when primitive data is "produced" and then immediately "consumed". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the producer/consumer nomenclature is quite appropriate here. More commonly you'll see "constructor" and "eliminator". It's the same duality though. That is, Right
is the constructor, and case
is the eliminator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An accessor semantically being sugar for a primitive case
.
getFoo = case _ of
{ foo } -> foo
|
||
#### Inlining and Inline Directives | ||
|
||
An inliner replaces a function's call site (e.g. stuff on the left-hand-side of the `=`) with its implementation (e.g. stuff on the right-hand-side of the `=`). This replacement means the function's body is duplicated and will appear at least twice in the resulting code: once in its original definition and once in the usage site. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nit, but the left-hand-side of =
is not a call-site. I think you can just say replaces a "function call", and then it would be more accurate.
- never inline it at all (i.e. the directive is `never`) | ||
- `9 + binaryPlus 1 2` | ||
- always inline it with no consideration for the number of arguments applied to it (i.e. the directive is `always`) | ||
- `9 + (\a b -> a + b) 1 2` | ||
- inline it only after at least one argument has been applied to it (i.e. the directive is `arity=1`) | ||
- `9 + (\ b -> 1 + b) 2` | ||
- inline it only after at least two arguments have been applied to it (i.e. the directive is `arity=2`) | ||
- `9 + ( 1 + 2)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this example is a little confusing because there's no difference here in practice between the arities here since they call is saturated anyway. To illustrate that you have to show some partial application. That is, with arity=2
something like binaryPlus 1
would not inline. However with arity=1
, you'd get \b -> 1 + b
.
```purs | ||
hasExpensiveLetBinding a b = do | ||
let | ||
someValue = expensiveComputation | ||
1 + someValue * b + a | ||
|
||
usage1 = hasExpensiveLetBinding 1 2 | ||
usage2 = hasExpensiveLetBinding 3 4 | ||
``` | ||
|
||
If we inline `hasExpensiveLetBinding` immediately, the `expensiveComputation` will be computed three times rather than once. In other words, it would be the same as writing the following in source code: | ||
|
||
```purs | ||
hasExpensiveLetBinding a b = do | ||
let | ||
someValue = expensiveComputation | ||
1 + someValue * b + a | ||
|
||
usage1 = (\a b -> do | ||
let | ||
someValue = expensiveComputation | ||
1 + someValue * b + a | ||
) 1 2 | ||
usage2 = (\a b -> do | ||
let | ||
someValue = expensiveComputation | ||
1 + someValue * b + a | ||
) 3 4 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is inaccurate. Inlining here has no effect on how many times expensiveComputation
is computed, it's always 2 because expensiveComputation
is deferred under a lambda.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be an example that demonstrates this effect while still being correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you should frame it not in terms of how expensive it is to compute/evaluate, but rather in terms of code size. That is, if expensiveComputation
(should be renamed) is a large expression that won't be reduced by the optimizer based on usages of a
or b
, then you've duplicated code without also reducing runtime overhead.
|
||
**To summarize, inlining for the sake of inlining is a great way to unnecessarily increase your program's bundle size**. | ||
|
||
#### Rewrite Rules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the name of this section is not inaccurate, it's really confusing if you are familiar with GHC rewrite rules. "Rewrite rules" don't appear anywhere in the source code. So, yes, technically there are rules for how the optimizer rewrites things, but I would steer very clear of calling it that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead I would probably just call this evaluation. The optimizer knows how to evaluate PureScript code, and will evaluate things at build time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would steer very clear of calling it that.
Ok, I'll do that.
will evaluate things at build time.
This isn't quite correct either as there are still some things it won't evaluate. For example:
mapWithIndex Tuple [ 1, 2, 3]
doesn't get evaluated to [ Tuple 0 1, Tuple 1 2, Tuple 2 3 ]
because no such "rule" exists. However, calling it something else like "evaluation rules", which may be more accurate, is too similar to "rewrite rules".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"evaluate things at build time" is not the same things as "evaluate everything that can possibly be evaluated at build time". It absolutely can evaluate anything in the PureScript language based on the foreign semantics you provide, it just doesn't always do that due to tradeoffs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All I'm saying is that we shouldn't invent terminology that doesn't exist in the project's code base, and we shouldn't co-opt terminology used in another similar, high-profile project. Evaluation is what's happening, which is why the main function is called eval
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this rendering?
purs-backend-es
can evaluate PureScript code, but it will only evaluate some expressions at build time. Typically, such expressions involve literal values (e.g.1
,{ foo: "string" }
). For example, primitive addition can be evaluated when its arguments are both literalInt
values (e.g.1 + 2
is evaluated to3
). The below code demonstrates this:
foo = do | ||
let | ||
a = { bar: 41 } | ||
1 + a.bar + 17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is somewhat confusing because +
is commutative. That is, if the optimizer was smart enough it could see that it's equivalent to a.bar + 18
. In reality, it only currently tries to evaluate around abstract terms associatively, and even then only to a fixed depth, because doing this in general is quadratic. It could probably handle commutative operations in a similar way though.
So the example is accurate, but it invites more questions from the reader that I don't think you want to answer here.
```purs | ||
ignoreArgs arg1 arg2 = do | ||
let | ||
a = { bar: 41 } | ||
1 + a.bar + arg1 + arg2 | ||
|
||
foo = ignoreArgs 8 9 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole series of steps is not quite accurate because the optimizer will optimizer ignoreArgs on it's own to
ignoreArgs arg1 arg2 =
42 + arg1 + arg2
Before it even does anything with foo
.
Fortunately, `purs-backend-es`' default inliners will see a record binding (i.e. a "producer") that is immediately accessed under the label `bar`. Thus, the corresponding value will be inlined: | ||
|
||
```purs | ||
ignoreArgs arg1 arg2 = do | ||
let | ||
a = { bar: 41 } | ||
1 + a.bar + arg1 + arg2 | ||
|
||
foo = do | ||
let | ||
a = { bar: 41 } | ||
1 + 41 + 17 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is inaccurate. The optimizer uses analysis to determine whether or not a binding should be inlined, and it will do that in lieu of creating a binding. So in this case, for ignoreArgs
, what it actually does is:
ignoreArgs arg1 arg2 = do
let
a = { bar: 41 }
1 + a.bar + arg1 + arg2
a
is a record constructor and is eliminated in all references, so it can be inlined.
ignoreArgs arg1 arg2 = do
1 + { bar: 42 }.bar + arg1 + arg2
The record constructor is immediately eliminated, so it can be reduced:
ignoreArgs arg1 arg2 = do
1 + 42 + arg1 + arg2
The operands of + are both known literals (up to some fixed depth, associatively), so it can be reduced.
ignoreArgs arg1 arg2 = do
43 + arg1 + arg2
First, think of a snippet of code you want to ensure is optimized. If you don't have a goal in mind, you will add inline directives that will unnecessarily bloat your code. | ||
|
||
Second, define a snapshot for that snippet. Without a small snippet of code, you won't be able to see what affects adding more directives may have. | ||
|
||
Third, look at the current JavaScript output of that snapshot. If it's already as optimized, there's no reason to add an inline directive. You're done. | ||
|
||
Otherwise, fourth, find the outermost function and add an inline directive for that. Because `purs-backend-es` operates on `CoreFn`, not PureScript source code, refer to the function in the outputted JavaScript to determine how many args the function takes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this just repeating the steps above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but each expands on the heading and provides more context, except... I thought there would be more to say for each step, but apparently not.
So, how should one determine if an inline directive needs to be added? One should always use the below methodology: | ||
|
||
1. Think of a snippet of code you want to ensure is optimized. | ||
2. Define a snapshot for that snippet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a snapshot and how does the reader define one? There's a lot of implicit knowledge here that makes no sense to most readers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the audience contributors to the project, or users of the backend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intended audience are users of the backend. I guess rather than 'snapshot', I should continue using 'snippet'. However, an example that clarifies what a good snippet would look like may help illustrate/define this term more.
🤔... |
I think in general, you should start from smaller examples and build up. I think you jump into let bindings and compound expressions too quickly. For example:
Can be reduced to
where
Can be reduced to
where
Can be reduced to
Cannot be reduced yet because in the expression
We can follow the same steps to reduce Then inlining directives are a way for authors to tell the optimizer when a top-level binding should be inlined apart from it's own heuristics if we know that doing so is advantageous. |
Ok, I rewrote the document with your feedback in mind. |
This PR seems to have been open for a very long time, though it seams to have all issues resolved. How can I help it to be merged? |
This is my attempt at explaining the what, why, and how of inline directives, so I am better equipped to fix #6 in a future PR (e.g. #21).