jeremyong / Selene

Simple C++11 friendly header-only bindings to Lua
zlib License
813 stars 117 forks source link

Low performace :( #93

Closed bagobor closed 9 years ago

bagobor commented 9 years ago

I created some simple bench suite for binding libraries: https://github.com/bagobor/cpp2lua-buindings-battle So you may notice that selene performance is significantly worse that other libraries.

bagobor commented 9 years ago

c_function_from_lua

jeremyong commented 9 years ago

Hmm well at a cursory glance, you aren't caching the selector which is requiring a string lookup every time you call the function (which the other libraries aren't doing). This is a pretty big deal when measuring at the millisecond level. It's entirely possible that there's a performance issue beyond that but it's hard to say without actually profiling more.

jeremyong commented 9 years ago

Another contributing factor is that when you do something like state["test"], you are actually creating a more powerful entity that can be assigned to, chained to other things etc. This is not a facility provided by other libraries so it's not a one to one comparison. I could modify the library to do things like lazily initialize setters to that field, but I don't like programming in order to make something perform well for a specific benchmark. More interesting would be a real world use case to motivate changes.

In your benchmark you have

nonius::benchmark("selene", [&sel_state] {
                int result = 0;
                for (size_t i = 0; i < NUM_ITERATIONS; ++i)
                    result += (int)sel_state["test"](42);
                return result;
            })

I recommend changing this to

nonius::benchmark("selene", [&sel_state] {
                int result = 0;
                auto test_fun = sel_state["test"];
                for (size_t i = 0; i < NUM_ITERATIONS; ++i)
                    result += (int)test_fun(42);
                return result;
            })

I have no idea how your benchmark will run as a result but I imagine it should be better.

jeremyong commented 9 years ago

Thanks for bringing this to my attention though :)

bagobor commented 9 years ago

Hi. Caching not always possible in realworld applications. So my point is to verify performance of available lua binding libraries to choose best one.

bagobor commented 9 years ago

I you found my works somehow usefull please give it a star ). Thanks. And any scenarios for bechmarks are welcome! )

bagobor commented 9 years ago

I fixed bug with luaintf after your remarks. thanks! But it looks like old time-proof luabind bites you hard :dash:

bagobor commented 9 years ago

Check update please. I add compassion with cached state runs. It still looks like there is some performance issue inside.

jeremyong commented 9 years ago

If I had to guess, it is because functional closures are used for the selector chaining. Lambdas were initially chosen when the design was a little different but are no longer necessary. It is pretty easily to implement the selector without the closures now which will make the selector object itself considerably smaller.

If anyone who sees this is interested, I can provide guidance on how to do this.

bagobor commented 9 years ago

I like Selene API and may found time to optimize it You can also reopen that issue - so any one else may also found you information useful.

jeremyong commented 9 years ago

I created #94 as a separate issue since it's a task on its own. There has been very little work done to optimize the library as it was initially written for teaching/demonstration purposes. Normally, I would sit down with a profiler to really fix it but I have been quite busy lately :)

bagobor commented 9 years ago

any way :+1: for lib.