Improve __index performance

[darkjacky]

Due to me not being a legendary Git user I accidentally closed the old pull request. https://github.com/Facepunch/garrysmod/pull/2272

Entity.__index Player.__index Weapon.__index

Cached the entity.GetTable result to make it about 50% quicker. Now keeping the if statements, there does not seem to be a performance penalty for it. I don't think there will be any issues with this change now.

[Astralcircle]

Throw errors when used on NULL.

] lua_run print(NULL.testvar)
> print(NULL.testvar)...

[ERROR] lua/1.lua:27: attempt to index a nil value
  1. __index - lua/1.lua:27
   2. unknown - lua_run:1

Also why change the function lookup function in the metatable and take the key from the table twice? You only made it slower by doing this.

[darkjacky]

First thing doesn't seem like an issue as NULL doesn't have a table so why would we be concerned about that? Worst case the best thing to do would be to return val or {} in the caching function this will generate a table that gets GCed instantly. Second thing, did you even try testing if what you are saying is true? Because I did test it and it makes about -0.0002438 seconds difference on 10000000 cycles on my computer, yes caching the value is worse. (Within margin of error. However it seems consistent)

[Astralcircle]

Error when using on NULL can cause compatibility issues. I'm sure there are many addons that do something like if not ent.MyFunc or not IsValid(ent) then return end

[darkjacky]

If an addon does ent.myfunc before IsValid it is doing it wrong on 2 levels. First the function should be in the metatable of Entity which wouldn't error. Second they did the order of operations wrong. IsValid should be the first thing you check. Again it would be trivial to just return val or {} to fix it but IMO if people are doing what you are suggesting they have bigger issues.

[wrefgtzweve]

Throw errors when used on NULL.

] lua_run print(NULL.testvar)
> print(NULL.testvar)...

[ERROR] lua/1.lua:27: attempt to index a nil value
  1. __index - lua/1.lua:27
   2. unknown - lua_run:1

Also why change the function lookup function in the metatable and take the key from the table twice? You only made it slower by doing this.

This would cause a lot of issues

[Astralcircle]

Ok, I tried to optimize GetTable this way - as a result, strange things started to happen in the form of missing or incorrect entity tables. One thing I can say for sure - it is not stable

[darkjacky]

Did you copy the files or copy the changes?

Either way I think this is dead in the water as it has quirks that can not be fixed as far as I can tell. One thing is the entity table not getting cleared after the ent is removed until collectgarbage is called. Not sure what is going on with Astralcircle if he has an error or what exactly is broken. That might be because of the return var or {}.

Try it with local tab = EntityTable[ self ] if ( tab and tab[ key ] ~= nil ) then return tab[ key ] end and remove the "or {}" from the EntityTable metatable __index function.

If that works we only have the collectgarbage issue left I think. Which might or might not be an issue depending on how people think about it.

[DatGuy64]

@darkjacky Hi, at first glance, everything appeared to be working correctly. However, after running on a live server with 50+ players for several hours, a serious issue started to occur. One or more entities occasionally become invalid, triggering the following error:

addons/fpp/lua/fpp/server/ownability.lua:198: attempt to index field 'FPPCanTouch' (a nil value)

This line refers to: https://github.com/FPtje/Falcos-Prop-protection/blob/master/lua/fpp/server/ownability.lua#L198

Even with an IsValid check at the beginning of the function, the problem still appears randomly and i don't know how to reproduce it, Sometimes it disappears on its own, only to reappear later.

[darkjacky]

@DatGuy64 I have changed something, rather than falling back to an empty table when the Entity table can't be read we return nothing which is in line with the original. So if it errors now it would have errored in the original version. This is now almost 100% the same as the original the only change being we cache the entity.GetTable result in Lua because it is faster. That does however mean if an entity, player or weapon is deleted the table won't be and will still be accesible until collectgarbage() is run. (Personally I see this as a positive but some people do not.)

[DatGuy64]

@darkjacky Sorry for the late reply. I've been using it on my server for two days, and I'm still getting the same error

[darkjacky]

Then this is death in the water. Unless your error is always happening even without this.

[DatGuy64]

It's not happening without this :/

[DatGuy64]

@darkjacky yo, i fixed my error by doing :

    local tab = ent:GetTable()

    tab.FPPCanTouch = tab.FPPCanTouch or {}
    tab.FPPCanTouchWhy = tab.FPPCanTouchWhy or {}

    local changed = tab.FPPCanTouch[ply] ~= canTouch or tab.FPPCanTouchWhy[ply] ~= reasons or tab.FPPOwnerChanged

    tab.FPPCanTouch[ply] = canTouch
    tab.FPPCanTouchWhy[ply] = reasons

maybe it can help you to find the issue