kikito / middleclass

Object-orientation for Lua
https://github.com/kikito/middleclass
MIT License
1.77k stars 190 forks source link

Optimize #58

Closed jojo59516 closed 2 years ago

jojo59516 commented 5 years ago

Hello. I found that a instance of a subclass always have a functional __index metamethod. But if I didn't override it's __index metamethod in baseclass, it should be a table. It will be more efficient. I optimized in fd25b1a. And I think conditional branches of a wrapped __index metamethod (returned by _createIndexWrapper()) could be decreased. I optimized in 63f83ea. All of specs are passed.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.1%) to 99.37% when pulling cc5cc98873122d55b3e183309fff0cd9d46aee90 on jojo59516:optimize into 6102f676c2da6bd395b3c32b1411bb70eea0fac9 on kikito:master.

jojo59516 commented 5 years ago

Obviously we all agree that a table index metamethod is better. Nevertheless I made a simple benchmark for this (with Lua 5.1.5 and LuaJIT 2.1.0-beta3 in interpreter mode). Before:

middleclass git/6102f67...*  
❯ lua performance/test.lua 
invoke unoverridden method      0.75
invoke overridden method        0.75

middleclass git/6102f67...*  
❯ luajit -joff performance/test.lua 
invoke unoverridden method      0.421875
invoke overridden method        0.421875

After:

middleclass git/master*  
❯ lua performance/test.lua         
invoke unoverridden method      0.359375
invoke overridden method        0.34375

middleclass git/master*  
❯ luajit -joff performance/test.lua
invoke unoverridden method      0.234375
invoke overridden method        0.234375

performance/test.lua:

local time = require 'performance/time'
local class = require 'middleclass'

local Base = class('Base')
function Base:f()
    return
end

local Derived = Base:subclass('Derived')
local obj = Derived()
time('invoke unoverridden method', function ()
    for i = 1, 1000 do
        obj:f()
    end
end)

function Derived:f()
    return
end
time('invoke overridden method', function ()
    for i = 1, 1000 do
        obj:f()
    end
end)
jojo59516 commented 5 years ago

Well, I added two test cases for checking instance's index metamethod being a table, in order to increase code coverage :)

kikito commented 2 years ago

Hey, many thanks for this. I apologize for the time it took me to review this. I think this contribution is good. I appreciate that you even added some extra tests to increase coverage :)

jojo59516 commented 2 years ago

Wow, thank you for the brilliant middleclass so much! :)