Improve TypeManager.function/TypeManager.virtual_function fetch speed by using cache.

[CookStar]

Entity has a mechanism to speed up the retrieval of dynamic_attributes/server_classes, but TypeManager itself does not. With this improvement, instances created by TypeManager can speed up the retrieval of function/virtual_function.

In virtual_function, I tried to improve the performance by using type_info and caching, but it did not improve the performance.

Test Code (CS:GO/Linux):

import time

from memory import find_binary
from memory import make_object
from memory.manager import CustomType
from memory.manager import TypeManager

""" TypeManager.function Test"""

# _ZN9CCSPlayer12RoundRespawnEv
signature = b"\x55\x89\xE5\x56\x53\x83\xEC\x1C\x8B\x5D\x08\xFF\x35\x2A\x2A\x2A\x2A\xE8\x2A\x2A\x2A\x2A\x83\xC4\x10\x84\xC0\x0F\x85\x2A\x2A\x2A\x2A\x8B\x13"

# Cache signature
find_binary("server", srv_check=False)[signature]

manager = TypeManager()

class Test(CustomType, metaclass=manager):
    _binary = "server"
    _srv_check = False
    _size = 0

    _spawn = manager.function(signature)

# Cache function
Test._spawn

s = time.perf_counter()
for i in range(100000):
    Test._spawn
e = time.perf_counter()
print(e-s, "Test._spawn")

# Cache function
test = Test()
test._spawn

s = time.perf_counter()
for i in range(100000):
    test._spawn
e = time.perf_counter()
print(e-s, "test._spawn")

l = [Test() for i in range(100000)]

s = time.perf_counter()
for i in range(100000):
    l[i]._spawn
e = time.perf_counter()
print(e-s, "Test()._spawn")

""" TypeManager.virtual_function Test"""

manager = TypeManager()

class Test(CustomType, metaclass=manager):
    blind = manager.virtual_function(551, (DataType.FLOAT, DataType.FLOAT, DataType.FLOAT))

pointer = Entity.find("player").pointer
test = make_object(Test, pointer)
test.blind

s = time.perf_counter()
for i in range(100000):
    test.blind
e = time.perf_counter()
print(e-s, "test.blind")

l = [make_object(Test, pointer) for i in range(100000)]

s = time.perf_counter()
for i in range(100000):
    l[i].blind
e = time.perf_counter()
print(e-s, "make_object(Test, pointer).blind")

Since the implementation has changed, the test results have also changed.
Output :

TypeManager.function Test
4.355441149324179 Test._spawn
4.830625455826521 test._spawn
4.801660872995853 Test()._spawn
↓
OLD: 0.02216128632426262 Test._spawn
OLD: 0.008792959153652191 test._spawn
OLD: 0.2989997826516628 Test()._spawn

NEW: 0.023133378475904465 Test._spawn
NEW: 0.29126785323023796 test._spawn
NEW: 0.293557733297348 Test()._spaw

TypeManager.virtual_function Test
0.5497394129633904 test.blind
0.5683586820960045 make_object(Test, pointer).blind
↓
OLD: 0.008566077798604965 test.blind
OLD: 0.5120681896805763 make_object(Test, pointer).blind

NEW: 0.46108797565102577 test.blind
NEW: 0.4727563001215458 make_object(Test, pointer).blind

[jordanbriere]

This is going to cause obj to leak forever.

[CookStar]

OK, that doesn't really make much sense. Forget about it.

[CookStar]

This was discarded. Here are the new test results.
Output :

TypeManager.function Test
4.355441149324179 Test._spawn
4.830625455826521 test._spawn
4.801660872995853 Test()._spawn
↓
0.023133378475904465 Test._spawn
0.29126785323023796 test._spawn
0.293557733297348 Test()._spaw

TypeManager.virtual_function Test
0.5497394129633904 test.blind
0.5683586820960045 make_object(Test, pointer).blind
↓
0.46108797565102577 test.blind
0.4727563001215458 make_object(Test, pointer).blind

[jordanbriere]

So there is no way to set MemberFunction to the instance. Can't we just use types.MethodType, for example, and set the method to the instance with a name like _function_name, with the exception for normal function call?

As you figured already, you would get the same leak with MethodType. In fact, any objects inheriting from Boost.Python.instance are never traversed for circular references. Basically, if any objects contained into its __dict__ strongly reference the object it belongs to it will be kept hostage by the circular references. For example, the following would leak:

ptr = alloc(1024)
ptr.self = ptr

Because the refcount of ptr will never reach zero, and will never be traversed by the cyclic collection. EntityMemFuncWrapper workaround this by storing a weak proxy instead of a strong reference:

Source.Python/addons/source-python/packages/source-python/entities/helpers.py

Line 104 in 12a24f4

self.wrapped_self = proxy(wrapped_self)

An easy workaround here, could be to simply set MemberFunction._this to obj's pointer instead of obj itself since that is all MemberFunction really need to forward the calls. Though, we should probably address the issue at the root, but I'm not sure of the overall implications.

EDIT: This seems to address the issue: ab33bc8

from gc import collect, is_tracked
from weakref import ref
from memory import alloc

ptr = alloc(1024)
# ptr should not be tracked, because it has no __dict__
assert not is_tracked(ptr)
ptr.self = ptr
# Now that ptr has a __dict__, it should be tracked
assert is_tracked(ptr)
r = ref(ptr)
# Our ref should be alive because ptr is still referenced
assert r()
del ptr
collect()
# Now that ptr has been deleted and collected, our ref should be dead
assert not r()

[jordanbriere]

Are there any other issues with this pull request?

I haven't tested, but based on the code, I can tell you that the _ptr() call on L628 is redundant since ptr is already a Pointer. That said, I think virtual_function could probably make great use of CachedProperty.

[CookStar]

I haven't tested, but based on the code, I can tell you that the _ptr() call on L628 is redundant since ptr is already a Pointer.

Done.

That said, I think virtual_function could probably make great use of CachedProperty.

Using CachedProperty leaks ptr/_this. Is there a way to use CachedProperty without causing leaks?

[jordanbriere]

Using CachedProperty leaks ptr/_this. Is there a way to use CachedProperty without causing leaks?

Technically this is what unbound is meant for. But now that I think about it, it seems flawed because this introduce a triangular reference that even ab33bc8 would theoretically not address.

[CookStar]

Yes, it does not work regardless of the value of unbound.

[jordanbriere]

Yes, it does not work regardless of the value of unbound.

Yeah, and it makes sense. Meaning that cached_result suffers from this regardless of 6d594fb. I don't remember specifically what testing I performed at the time that led me to believe otherwise but, if I had to guess, I probably used Player.is_bot with non-cached objects. This is misleading because it inherits from Entity.__hash__ meaning that the same value is re-used for every objects that are allocated for the same player.