Skip to content

Conversation

@oxinabox
Copy link
Member

@oxinabox oxinabox commented Dec 4, 2020

I recall this was causing problems for @dpsanders a while ago.
I am not sure if it still is, but it seems reasonable to do this.

Copy link
Member

@mzgubic mzgubic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My basic understanding of @inline is that it removes the function call and just copies the function body where it is called, thus improving time performance while worsening the space requirements. The rule of thumb seems to be that short function should be inlined. Are there any additional considerations due to metaprogramming?

@oxinabox
Copy link
Member Author

oxinabox commented Dec 4, 2020

Basically right.
Compile time increases somewhat, performance increases significantly.
This is because the optimizer is worse than O(n^2) in time complexity where n is the number of IR statements in the function being optimzied, and so is increased by inlining more statements inplace.
But (roughly speaking) the optimizer also only runs one function at a time, so inlining it lets it work to optimize more code.

Julia normally inlines things pretty agressively even without the @inline annotation.
But there is a cost heuristic where it decides that it doesn't want to inline any more
@inline forces it to inline (except in some recursive cases), telling it to ignore its heuristic cost estimate of if it is worth it.

Since pullbacks are short functions they normally get inlined on their own, but @dpsanders was seeing that for more complicated types used for interval arithmetic they were sometimes not, which was causing significant slow downs.

Are there any additional considerations due to metaprogramming?

There are not.

@mzgubic
Copy link
Member

mzgubic commented Dec 4, 2020

Thanks for the details, I didn't know about the optimiser time complexity. Should we bump the version number?

@nickrobinson251
Copy link
Contributor

nickrobinson251 commented Dec 7, 2020

fixes JuliaDiff/ChainRules.jl#248

@oxinabox oxinabox merged commit a1a8da5 into master Dec 7, 2020
@oxinabox oxinabox deleted the ox/inline branch January 11, 2021 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants