Skip to content
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

Simple version of @btime with automatic interpolation #118

Open
jlperla opened this issue Sep 21, 2018 · 16 comments
Open

Simple version of @btime with automatic interpolation #118

jlperla opened this issue Sep 21, 2018 · 16 comments

Comments

@jlperla
Copy link

jlperla commented Sep 21, 2018

I know this has been brought up before (e.g. #65 ) and others, but let me make a slightly different suggestion. The current @btime and @benchmark are great, but what about a simple @rdeits made a similar suggestion in #65 (comment)

The issue is that the vast majority of benchmarking I do is of the form @btime f(a,b) and no matter how many times I have read the manual, I will always forget to do it as @btime f($a, $b). Not to mention that it is harder to read or flip benchmarks on/off easily for a function you are working on.

So my suggestion is a macro @bfunction or whatever, which only accepts a function as head and automatically interpolates all arguments. i.e.

@bfunction f(a,b) -> @btime f($a, $b)

Is that sort of thing possible to do with the macro expansion model in Julia? If I was able to get a PR would you consider merging it, or do you disagree with the feature?

@rdeits
Copy link

rdeits commented Sep 30, 2018

Yeah, I'm definitely tired of (a) remembering to do the interpolation myself and (b) explaining it to others. I think the easiest thing to do is essentially what I proposed over in #65 and create a new package which just provides its own @btime, @benchmark etc. that just "do the right thing", i.e. do the interpolation automatically. Then people who want automatic interpolation can just do using MyPackage: @btime instead of using BenchmarkTools: @btime.

I just tried writing an implementation of this, and it turned out to be pretty simple given the available tools:

import BenchmarkTools
using MacroTools: postwalk

function interpolate(expr)
    postwalk(x -> x isa Symbol ? Expr(:$, x) : x, expr)
end

macro benchmark(expr, args...)
    quote
        BenchmarkTools.@benchmark($(interpolate(expr)), $(args)...)
    end
end

it's likewise pretty trivial to implement @btime, @belapsed, etc.

Any thoughts on a name? LocalScopeBenchmarks.jl is reasonably clear but a bit long-winded.

@jlperla
Copy link
Author

jlperla commented Sep 30, 2018

Why a new package rather than a new macro within this package? Is there resistance to merge something of that sort, or something more.

Regardless, if it is in a new package (and your name sounds fine) then I don't think it should clashes of named macros with this package, just in case something uses both, or gets confused on behaviour

@rdeits
Copy link

rdeits commented Sep 30, 2018

I mean it's really up to the maintainers whether they want to add more functionality to this package or to keep it separate. But one way or another I'm definitely done with @btime f($x) :-p.

@jrevels
Copy link
Member

jrevels commented Oct 1, 2018

I'm all for this if somebody can come up with a correct implementation that doesn't end up being too magical. AFAICT the proposed implementation above is unfortunately broken, for example it would try to mark setup variable usages with $. I'm sure there are other edge cases as well, it's a tricky transformation to do automatically.

It's basically just a matter of switching the "default" scope of variables. Right now, by default, variables in benchmark kernels just follow normal Julia scoping rules. One can then use $ to tell BenchmarkTools to do some extra processing to interpolate the value and bind it to a variable in the harness scope, and then pass it to the benchmark kernel as an argument. We could just reverse that if people find it more convenient, it just introduces some extra edge cases and requires the user to understands a slightly more complicated scoping model.

Either way, normal usage should probably see variables being $ half the time and follow normal scoping rules the other half of the time, so BenchmarkTools currently just implements the simpler choice by default.

Honestly, I think the root of the problem is just that scope is a very important part of benchmarking in Julia, and one cannot author a well-defined benchmark (or correctly interpret results) without explicitly understanding the scope of data manipulated by the benchmark. This comprehension barrier is going to exist regardless of which syntax is chosen.

@jlperla
Copy link
Author

jlperla commented Oct 1, 2018

Honestly, I think the root of the problem is just that scope is a very important part of benchmarking in Julia, and one cannot author a well-defined benchmark (or correctly interpret results) without explicitly understanding the scope of data manipulated by the benchmark.

I think you are exactly right. And that is plenty of reason that it shouldn't replace @btime, but be something distinct (and likely limited in scope/features).

The issue is that @btime is being used for things other than well-defined benchmarks and instead as an exploratory tool and rough check on performance. For most users, this is what they are looking for, and they are willing to sacrifice all sorts of features as long as it gets it right most of the time. This exploratory use-case also tends to be one which is toggled on/off on a function, and the manual changing of the $ is easy to forget.

Right now the heuristic people like myself are using is "always put $ in front of everything... unless I forget".

@rdeits
Copy link

rdeits commented Oct 1, 2018

Yeah, I noticed the issue with setup right after I posted that. Fortunately, it's not too bad to work around: rather than interpolating everything, I just add entries to setup for any local symbols (except those symbols which already have entries in setup). So if we have:

@btime f(x, y) setup=(x = 10)

I can produce:

@btime f(x, y) setup=(x = 10; y = $y; f = $f)

I've got that version mostly working here: https://github.com/rdeits/LocalScopeBenchmarks.jl/blob/b7407351be6f99be6116ce1f28514278f65c3fac/src/LocalScopeBenchmarks.jl

@jlperla
Copy link
Author

jlperla commented Oct 28, 2018

This keeps coming up when trying to get beginners up and running, and suggest to them to explore things with simple benchmarks. Splicing is not as obvious to people as seasoned Julia may think.

Are they any thoughts on whether we need a new benchmarking library, or try to add one here? It sure would simplify things (and cut down on discourse "why isn't this fast" questions)

@rdeits
Copy link

rdeits commented Oct 28, 2018

If you're willing to try out LocalScopeBenchmarks.jl (linked above), I think it might help

@jlperla
Copy link
Author

jlperla commented Oct 29, 2018

Do you mean to try it out with the intention of it becoming a supported package in the medium-term, or to try out the patterns to see if it works (and could hence be integrated back into BenchmarkTools)? My main interest for students is to teach them to use packages that I know are going to be supported for a while.

@rdeits
Copy link

rdeits commented Oct 30, 2018

More the latter, I'm afraid. I'm going to keep playing with it myself to see if it's the right interface, but I'm not quite to the point of a long-term commitment.

@jrevels
Copy link
Member

jrevels commented Oct 30, 2018

@jlperla If students are still having trouble understanding benchmark scope (which is indeed a complicated thing in Julia), perhaps the easiest way to avoid the problem is to just teach them to always use the normal setup workflow, which translates better to other language's benchmarking tools e.g. timeit in Python. Teaching the fairly Julia-specific variable interpolation feature first seems like skipping a step pedagogically, if they've never worked with other software benchmarking tools.

@jlperla
Copy link
Author

jlperla commented Nov 10, 2018

Sorry for the delay. Thanks @jrevels I think you may misunderstand this usage. It is not about teaching people how to do real benchmarking (which requires sufficient care that I think variable interpolation is a reasonable thing to do).

The main motivation for this is pure exploration for performance in code, with no intention to do fancy benchmarks. i.e. you have a bunch of functions and want to time one of them, so if you just throw a @btime or @localbtime or whatever macro in front of the function, then you would get a good timing on it robust to compile time/noise. The @time is not really useful for this, whatever its intentions, so everyone uses @btime.

@jlperla
Copy link
Author

jlperla commented Nov 22, 2018

If anyone is curious, try flipping through https://discourse.julialang.org/search?q=btime to see 2 things: (1) how essential this package is for the Julia ecosystem; and (2) how many times @btime is used with someone forgetting to interpolate variables when it really matters.

@jlperla
Copy link
Author

jlperla commented Aug 23, 2019

@jrevels @rdeits

Hi. We are almost a year (and countless mistakes of me and others forgetting $ in front of variables when doing simple benchmarking) and wanted to see if you would be willing to consider this again. The motivation: it is too easy to forget escaping, but for many use cases you unconditionally want to escape. All of the links in https://github.com/rdeits/LocalScopeBenchmarks.jl#introduction were collected a year ago, but I bet there are many more since.

To be precise, the proposal is to add simple, convenient macro to the package (i.e. not changing the behavior of any existing functions!) for a "local" version of btime. Lets call it @lbtime or @localbtime for now, which has no options or fancy behavior, but fulfills 95% of simple benchmarking needs for people just exploring performance. That is take the basic pattern

f(x) = x^2
y = 1:5
@btime f.($y)

Then the following would be equivalent

@lbtime f.(y)

Again, this is not in any way intended for serious benchmarking. It just makes it less error-prone to use BenchmarkTools for simple cases. The code could be based off of https://github.com/rdeits/LocalScopeBenchmarks.jl/blob/master/src/LocalScopeBenchmarks.jl or even simplified further.

@jlperla
Copy link
Author

jlperla commented Aug 30, 2019

@ajozefiak I discussed this with @jrevels and here is the basic strategy.

Right now, @btime is a thin wrapper for @benchmark. In reality, it doesn't add much other than throwing some of the output away. People doing real benchmarking where they need full control should (and frequently will be) using @benchmark. With it, you get full control over the scope that code is executing, for example.

What is discussed above was to create a @lbtime macro which interpolates for @btime... but we instead should think of it "replacing" the current @btime so that it replaces the local scope as part of its main features. There is a chance this will break some use of @btime, but we can try to avoid it, and it may be better for the longrun.

So do the following in a PR:

  • Create a @lbtime macro, starting with the code from @rdeits that calls the current @btime function and does the interpolation
  • It looks like he may have fixed up an error or two from the previous feedback, so we need to try to find cases where it simply does not work.
  • If all code seems to work, then we can rename @btime to @_btime and @lbtime to @btime.... or something along those lines.

To be clear on finding places where it breaks, you may not have too many troubles if it doesn't use the setup option, but consider the following (where @btime is the old function)
For the basic problem:

@lbtime f(x, y)

Should be equivalent to

@btime f(x, y) setup=(x = $x, y = $y)

Or probably even (though you should check that this doesn't cause any issues)

@btime f(x, y) setup=(x = $x, y = $y, f = $f)

Keep in mind that the setup argument is passed to the @benchmark so this should be managed... They may have used it already:

@lbtime f(x, y) setup=(x = 10)

Should be equivalent to

@btime f(x, y) setup=(x = 10, y = $y, f = $f)

Similarly, if someone has already escaped a variable, then we need to make sure it doesn't break - since a lot of users of @btime are already escaped.

@lbtime f(x, y, $z)

is equivalent to

@btime f(x, y, z) setup=(x = 10, y = $y, f = $f, z = $z)

Or if it is easier,

@btime f(x, y, $z) setup=(x = 10, y = $y, f = $f)

Good luck with learning the julia AST! Soon it will be time to go one step further and get your hands dirty with the IR.

@jlperla
Copy link
Author

jlperla commented Aug 30, 2019

A few other tests I thought of:

@lbtime f(x, $(y+1))

Should probably be the same as

@btime f(x, $(y+1)) setup = (f = $f, x = $x)

which might change the approach to the @lbtime f(x, y, $z) case. If you, in general, just leave any interpolations in the AST in place, it might be the way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants