cleaner lookup code

[matthewleon]

The lookup function was particularly strangely written, perhaps a holdover from some earlier version of the type system that required more hinting?

I'd be curious to benchmark this. I imagine that there might be a very slight performance improvement (pre-optimization) due to partially applying the lookup functions at the outset.

[garyb]

@natefaubion should comment on this probably. I think it was written the way it is to improve the instance lookup overhead.

We should probably add benchmarks to ensure we don't accidentally de-optimise things like this 😄

[matthewleon]

Ah, yes, I can see that being the case for the usage of comp. Moving k out still makes sense, though, no?

[natefaubion]

Yeah, I think these changes are good. You could probably pull out comp in the other lookups as well.

[hdgarrood]

Having a benchmarking library suitable for core libraries to depend on would be amazing. I think benchotron might have a few awkward dependencies. Then again if it's only going to be listed in devDependencies maybe that's not a problem.

[hdgarrood]

I just checked and all of benchotron's dependencies are either core libraries or under purescript-node. The only awkward dependency is benchmark.js which you currently have to obtain separately eg via npm. That's fairly easy to fix though, I think, by including it in an FFI file.

There's also the issue of circular dependencies but I think that can be worked around without too much difficulty.

[paf31]

Something at the level of purescript-assert in core might be nice. Even something which just runs a function N times and prints the mean and std. dev. of times would be kind of nice to have. I find I don't need graphs or other niceties for many benchmarking tasks I do.

[paf31]

I think benchotron is great, by the way, I just find that having big devDependencies can be a little bit of a pain when we do a major version bump in core, since we need to come back after those are updated and make sure the tests still work.

[paf31]

The simplest thing we could do is to just add support for Console.time to purescript-console.

[matthewleon]

The only awkward dependency is benchmark.js which you currently have to obtain separately eg via npm. That's fairly easy to fix though, I think, by including it in an FFI file.

The dep on microtime involves C compilation. I think you can forgo it at the expense of lower timer resolution.

[matthewleon]

Another possibility is a separate repo for, say, collection benchmarks.

[matthewleon]

You could probably pull out comp in the other lookups as well.

Done. I do wonder if any of the optimization tools out there would do this automatically, though. It feels like a bit of a hack, no?

[natefaubion]

It is most definitely a hack, and can yield significant improvements. Ideally the compiler would pull it out in a similar manner. I don't think any of the bundlers pre-apply TC dictionaries.

…

On Jun 2, 2017, at 9:33 AM, Matthew Leon ***@***.***> wrote: You could probably pull out comp in the other lookups as well. Done. I do wonder if any of the optimization tools out there would do this automatically, though. It feels like a bit of a hack, no? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[matthewleon]

Ideally the compiler would pull it out in a similar manner.

Related, I think: purescript/purescript#687

[garyb]

Related, I think: purescript/purescript#687

Yeah exactly, the issue originally had a less general name, specifically talking about typeclass code :)

[paf31]

Thanks!

We can benchmark before this gets released if necessary. @matthewleon Were you able to do any simple benchmarks already?

[paf31]

I'd like to suggest we use this library for simple benchmarks in core (after moving it into core, of course). The main issue right now is that it doesn't account for outliers very well.

[matthewleon]

Were you able to do any simple benchmarks already?

Not yet, sadly. It is my next priority, though. Early this week I should have something rudimentary up and running.