Hacker News Reader

Comment by rpcope1 | Hacker News Reader

If you're going to optimize with closures, why not go a step further and pull the conditional out of the closure? You could instead have a conditional in the function that produces the right closure based on mode rather than constantly re-checking mode every single time you call the closure.

Also, maybe it would be worth discussing building temporary function variables in outer scope that resolve the methods before the closure is called (i.e. my_list_append = my_list.append). There was no opportunity to do this in example, but it's worth discussing.

Finally, dict access seems like a bad idea if you're this pressed for performance, especially since you only need one value -- maybe better just to encourage explicitly passing in a parameter.

View on HN

This same oddity (checking a mode flag rather than returning one of two different implementations) shows up in their original code example for "Classes": that mode field and dictionary configuration should not "pass most code reviews" unless the people you are showing it to do not know much about object orientation... the description of the problem they are trying to solve (having different implementations that provide the same interface) should have been handled via polymorphism.

(And as they care about performance, and have tested PyPy, they are hopefully using it, and so the class-based solution is just as fast as the closure-based one, and is going to allow for better decoupling of filter modalities; though, as someone who does a lot of functional programming, the closure-based approach does resonate with me.)

As I mentioned in a comment on Jaime's response (https://wrongsideofmemphis.wordpress.com/2015/05/08/optimise...), we actually do quite a bit more than this in our production code. The blog post here is meant to demonstrate the point, not to show our exact production code.

In particular, I wanted all 3 examples to use nearly identical inside the filter function, to isolate the differences just between the ways of accessing data in the benchmark results, and to show that closures are an easy way to gain some performance in hot spots (in CPython at least).

I get that you wanted to simplify the example, but the example you've written really just doesn't make much sense, so it's hard to understand your point.

The API in the example is really bad. Accepting a dictionary, only to require a specific key in the dictionary, is the worst of both worlds. But then if you use .get to access the dictionary body instead of the attribute access, you'll take on additional performance penalties, and other solutions will start to compete.

This was the first thing that jumped out at me too when I saw the code. I kept expecting them to optimize that- it would have been cleaner too. Very surprised when they wrapped it all up leaving the if statement still inside the closure.

> If you're going to optimize with closures, why not go a step further and pull the conditional out of the closure? You could instead have a conditional in the function that produces the right closure based on mode rather than constantly re-checking mode every single time you call the closure.

I tend to try and do that in any language as soon as a single conditional means a change in behaviour in different locations, regardless of optimization concerns. It makes for (IMHO) more robust code and much easier testing (though you need more mocks).

Reply on news.ycombinator.com