conversion error on index_from_pointer

Please post any questions about developing your plugin here. Please use the search function before posting!
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

conversion error on index_from_pointer

Postby velocity » Mon Nov 22, 2021 5:43 pm

This function is very heavy on the server performance is there a better way to handle index_from_pointer conversion error besides 'try' 'except'? Maybe this is something should be handled on c++ side, by returning None instead of conversion error?


Syntax: Select all

server = memory.find_binary('server', srv_check=False)

PassServerEntityFilter = server[b'\x55\xB8\x01\x00\x00\x00\x89\xE5\x83\xEC\x38\x89\x5D\xF4'].make_function(
Convention.CDECL,
[DataType.POINTER, DataType.POINTER],
DataType.BOOL
)


@PreHook(PassServerEntityFilter)
def _pre_pass_server_entity_filter(args):
try:
p2 = index_from_pointer(args[1])
except ValueError:
return

p1 = index_from_pointer(args[0])



Without try except I get this error:
ValueError: Conversion from "Pointer" (<_memory.Pointer object at 0xeb7d85f0>) to "Index" failed.
User avatar
Ayuto
Project Leader
Posts: 2193
Joined: Sat Jul 07, 2012 8:17 am
Location: Germany

Re: conversion error on index_from_pointer

Postby Ayuto » Mon Nov 22, 2021 8:39 pm

I guess the try/except is required, because the hook also gets called for non-networked entities.

However, you are right. Raising an exception is quite expensive, which is shown by a quick test (internally, we are working without exception, but with true/false):

Syntax: Select all

object IndexFromPointer2( CPointer *pEntityPointer)
{
unsigned int result;
if (!IndexFromPointer(pEntityPointer, result))
return object();
return object(result);
}

Syntax: Select all

import time

from _entities._helpers import index_from_pointer, index_from_pointer2
from memory import Pointer
from _entities._entity import BaseEntity

ITERATIONS = 100000

def test(ptr):
now = time.time()
for x in range(ITERATIONS):
try:
index = index_from_pointer(ptr)
except ValueError:
pass

print('index_from_pointer ', time.time() - now)

now = time.time()
for x in range(ITERATIONS):
index = index_from_pointer2(ptr)
if index is None:
pass
else:
pass

print('index_from_pointer2', time.time() - now)

print('Test with an NULL pointer')
test(Pointer())

print('Test with an invalid pointer (123)')
test(Pointer(123))

print('Test with a non networked entity pointer')
test(BaseEntity.find_or_create('math_counter')._ptr())

Code: Select all

Test with an NULL pointer
index_from_pointer  1.4640326499938965
index_from_pointer2 0.02597975730895996
Test with an invalid pointer (123)
index_from_pointer  2.9059977531433105
index_from_pointer2 1.2610502243041992
Test with a non networked entity pointer
index_from_pointer  2.911987781524658
index_from_pointer2 1.258424997329712
The funny thing is that there was an optional argument "raise_exception" in the past in our conversion functions. If you set it to False, None was returned instead of raising an exception. But then we decided to only offer one way, which was returning None. But the concerns in the announcements were very valid, which lead me to implement the try/except variant:
viewtopic.php?f=5&t=991&sid=701c7077e57bbfba50522726e051b716#p6394
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Mon Nov 22, 2021 9:24 pm

Isn't the point of having that extra functionality such as the 'raise_exception' choice for exactly functions like this. Even if the general answer is to use try/except. This is a good example, where performance outweigh elegance.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Mon Nov 22, 2021 10:04 pm

Maybe this issue is along the lines of this: https://github.com/Source-Python-Dev-Te ... issues/332 . It probably would be better all together handling the PassServerEntityFilter on the C++ side and then in python you have function arguments like in the sourcemod implementation.

SourceMod:

Syntax: Select all

public Action CH_PassFilter(int ent1, int ent2, bool &result)
{
// Code
}


Python:

Syntax: Select all

def pass_server_entity_filter(index1, index2): 
# ..code ...
Last edited by velocity on Thu Nov 25, 2021 11:20 am, edited 1 time in total.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Tue Nov 23, 2021 2:07 am

You could use ServerUnknown to bypass the exception handling and do the validation yourself:

Syntax: Select all

from entities import ServerUnknown

def idx_from_ptr(pointer):
try:
unk = ServerUnknown._obj(pointer)
except RuntimeError:
return None
if unk is None or not unk.is_networked():
return None
return index_from_pointer(pointer)


Syntax: Select all

from time import time
from entities.constants import WORLD_ENTITY_INDEX
from entities.entity import BaseEntity
from entities.helpers import index_from_pointer

ptr = BaseEntity(WORLD_ENTITY_INDEX).pointer

t = time()
for i in range(100000):
index = idx_from_ptr(ptr)
print('Networked:', time() - t)


ptr = BaseEntity.find_or_create('math_counter')._ptr()

t = time()
for i in range(100000):
index = idx_from_ptr(ptr)
print('Non-networked:', time() - t)


Code: Select all

Networked: 0.05878875732421875
Non-networked: 0.0498604679107666


Or, you could simply maintain a dictionary of addressindex and look it up:

Syntax: Select all

from core import AutoUnload
from filters.entities import BaseEntityGenerator
from listeners import on_entity_created_listener_manager
from listeners import on_entity_deleted_listener_manager

class EntityIndexes(AutoUnload, dict):
def __init__(self):
super().__init__()

for base_entity in BaseEntityGenerator():
self.on_entity_created(base_entity)

on_entity_created_listener_manager.register_listener(
self.on_entity_created)
on_entity_deleted_listener_manager.register_listener(
self.on_entity_deleted)

def on_entity_created(self, base_entity):
try:
self[base_entity.pointer.address] = base_entity.index
except ValueError:
self[base_entity.pointer.address] = None

def on_entity_deleted(self, base_entity):
self.pop(base_entity.pointer.address)

def _unload_instance(self):
on_entity_created_listener_manager.unregister_listener(
self.on_entity_created)
on_entity_deleted_listener_manager.unregister_listener(
self.on_entity_deleted)

entity_indexes = EntityIndexes()


Syntax: Select all

from time import time

t = time()
for i in range(100000):
index = entity_indexes.get(4215362, None)
print('Invalid:', time() - t)

from entities.entity import Entity

address = Entity(WORLD_ENTITY_INDEX).address

t = time()
for i in range(100000):
index = entity_indexes.get(address, None)
print('Valid:', time() - t)


Code: Select all

Invalid: 0.012956951141357422
Valid: 0.013957521438598633
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Tue Nov 23, 2021 2:52 pm

Great idea with storing the address-index relationship in a dict, I'll definitely try this. We can maybe even go one step further and store the address-entity relationship, so I don't have to do a second dict lookup for the entity after getting the index, as I am already using EntityDictionary to store the entities. Also, what does AutoUnload do exactly?
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Tue Nov 23, 2021 5:10 pm

velocity wrote:Also, what does AutoUnload do exactly?

It calls EntityIndexes._unload_instance when your plugin is unloaded so that the callbacks are being unregistered.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Thu Nov 25, 2021 7:54 pm

Better, quite good actually, but just not C++ performance. It is still not fast enough if there are too many players and compared to SourceMod implementation, I'm telling you that there are "no performance issues" in sourcemod with this.


I optimized it a bit further by using

Syntax: Select all

if address not in dict: #..code#
instead of

Syntax: Select all

dict.get(address, None)
, this is actually slower than 'not in' lookup, because it takes time to lookup the get attribute first. Even so, I have to do a second comparison anyways (if key is not None). At least according to this https://stackoverflow.com/questions/39582115/python-dictionary-lookup-performance-get-vs-in. I also did run some tests myself.

Tests:

Syntax: Select all

from time import time

A = {'a':0, 'b':0, 'c':0}
b = 'd'

t = time()
for _ in range(10000000):
if b not in A:
continue
result = A[b]
print(time()-t)

t = time()
for _ in range(10000000):
result = A.get(b, None)
if result is None:
continue
print(time()-t)


.get() = 0.6377964019775391s
not in = 1.6540522575378418s


Is it possible for this function to be implemented in C++? What you guys think. Right now it is just impossible to use in sourcepython.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Thu Nov 25, 2021 8:55 pm

velocity wrote:I optimized it a bit further by using

Syntax: Select all

if address not in dict: #..code#
instead of

Syntax: Select all

dict.get(address, None)
, this is actually slower than 'not in' lookup, because it takes time to lookup the get attribute first. Even so, I have to do a second comparison anyways (if key is not None). At least according to this https://stackoverflow.com/questions/39582115/python-dictionary-lookup-performance-get-vs-in. I also did run some tests myself.

Tests:

Syntax: Select all

from time import time

A = {'a':0, 'b':0, 'c':0}
b = 'd'

t = time()
for _ in range(10000000):
if b not in A:
continue
result = A[b]
print(time()-t)

t = time()
for _ in range(10000000):
result = A.get(b, None)
if result is None:
continue
print(time()-t)


.get() = 0.6377964019775391s
not in = 1.6540522575378418s



While this is true for when the key doesn't exist, you end up querying it twice for when it does because dict.__contains__ resolve the value associated with the key to check its presence, and then you retrieve it again through dict.__getitem__. The length of your dictionary will also affect the lookup speed, and while it may be faster for a dictionary that only contains 3 keys, you will get different result for a larger container. To be fair, you should rather use a try/except and catch KeyError which would be the fastest approach because the chance your hook gets called with an invalid pointer you don't have in your container is statically next to null since you keep it synced at all time. Just for science, take a look at this:

Syntax: Select all

from time import time
from random import randint

d = dict()
for i in range(2048):
k = randint(0, 2147483647)
d[k] = i

print('Key exists:')
t = time()
for i in range(10000000):
if k not in d:
continue
v = d[k]
print('\tIN:', time() - t)

t = time()
for i in range(10000000):
v = d.get(k, None)
print('\tGET:', time() - t)

t = time()
for i in range(10000000):
try:
v = d[k]
except KeyError:
continue
print('\tTRY:', time() - t)


print('Key does not exists:')
k = 'foo'

t = time()
for i in range(10000000):
if k not in d:
continue
v = d[k]
print('\tIN:', time() - t)

t = time()
for i in range(10000000):
v = d.get(k, None)
print('\tGET:', time() - t)

t = time()
for i in range(10000000):
try:
v = d[k]
except KeyError:
continue
print('\tTRY:', time() - t)


Code: Select all

Key exists:
        IN: 1.5538604259490967
        GET: 1.5824986934661865
        TRY: 1.0551846027374268
Key does not exists:
        IN: 0.8257911205291748
        GET: 1.8011598587036133
        TRY: 3.390928030014038


velocity wrote:Is it possible for this function to be implemented in C++? What you guys think. Right now it is just impossible to use in sourcepython.
I'm personally not opposed to it, except maybe for the fact it adds an additional signature to maintain for all games. But other than that, please feel free to make a PR.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Fri Nov 26, 2021 11:14 am

I see alright, I'll try both see how many calls there are made from the function itself,

Yeah.. I would have done a PR if I could :smile: I don't have that kind of overview over SP src yet, this would be a project in itself. But if someone else could do it that would be great :rolleyes: :cool: Because of this maybe it is good idea to issue on the github.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Fri Nov 26, 2021 7:01 pm

I have analyzed the function where the calls come from. I know this is a rough estimate as the passentityfilter arguments change on different game behavior, but when jumping around etc, this is it (with 1 player):

15 seconds of PassEntityFilter

Syntax: Select all

TOTAL_FUNCTION: 61194 times
DICT_NOT_IN: 61194 times
DICT2_NOT_IN: 20398 times
CONVERSION_ERROR: 30597 times
SAME_ARGS: 10199 times


DICT and DICT2 are what we discussed it holds the address of some specific entities, for example, player instance.
CONVERSION_ERROR is the try/except for index_from_pointer.
SAME_ARGS is when args[0] == args[1] case, which is usually never interesting, so these can be skipped.
TOTAL_FUNCTION is total function calls.

Roughly OVER half of the calls to the function in general, if we add up SAME_ARGS + CONVERSION_ERROR are wasted calls and can be skipped.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Fri Nov 26, 2021 7:20 pm

velocity wrote:TOTAL_FUNCTION is total function calls.

It doesn't seem too bad, to be honest. I mean, that number may seems huge but when you divide it by 15, and then by the frame rate, you ends up with few dozens call a frame which is not the worst I've seen.

That said, you should provide more details and context of your specific use case, because there may be many ways to optimize if you ask yourself the right questions.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Fri Nov 26, 2021 7:28 pm

I get what you mean, but I think its a trap only looking at the total function calls, because they grow exponentially with more players and of course even more so when let's say player1 is colliding with player2 (game specific behavior). What I wanted to point out is ratio between unnecessary calls to necessary calls (not for the engine perhaps) but for people hooking the function, each time there is a conversion error or arguments are equal, I cannot think of a situation where that would be useful.

Nonetheless, because of this, yes you are right, it might be more forgiving than Set Transmit, for example, because the amount of calls is based on game specific behavior and not just every tick, but imagine using this together with Set Transmit then you up for trouble :)
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Fri Nov 26, 2021 7:55 pm

SetTransmit is not called every tick, but only when the entities are in PVS and not already flagged as never emitted. But yeah, it can get rather noisy. However, both are quite different and unless I'm mistaken about what your goal is here I don't think you are using the right tools.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Fri Nov 26, 2021 8:02 pm

Alright my use case is this:

Trying to recreate this in SourcePython:
A short time summary, because the code is rather messy:
In the gamemode trikz players complete levels in teams of 2. If there are more than 1 team on a level, they have to wait for each other to finish it. But by hooking the entities in the map, it is possible to make entities, for example, the entity "func_breakable" (wall) invisible for some and others not with Set_Transmit, but Set Transmit is not enough as players still collide with entity, here you use the PassSeverEntityFilter to disable the collision when colliding with the invisible entity. So this is basically to make it possible for multiple teams to complete the same level simultaneously.

I did manage to do so at some point in SourcePython but I discarded the idea after I noticed lag with more than 4 players were in the server.
Last edited by velocity on Mon Dec 06, 2021 3:18 pm, edited 4 times in total.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Fri Nov 26, 2021 8:18 pm

Then yes, my assumptions were right and you are not using the right tools. The first questions you have to ask yourself when hooking a function is;

  1. Do I want to override a parameter with my own?
  2. Do I want to prevent the original function from doing something?
  3. Do I only want to override its result under certain circumstances?

If you answered No, No and Yes which I assume you did then; you don't need a pre but a post hook. What you are currently doing is re-implementing checks the original function already does and then you let it do them again for calls you don't want to override. What you really want here is the following:

Syntax: Select all

@PostHook(PassServerEntityFilter)  
def _pre_pass_server_entity_filter(args, result):
if not result:
return

...


Because you only want to override calls that passed, so there is really no point into processing calls that already does not.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Fri Nov 26, 2021 8:32 pm

Yeah that actually makes sense hah :) Thank you. I think this it. Oh well, hopefully our discussion weren't in vain as the idea with storing the addresses as entity/index relationship was genius - very informal and good. I'm gonna implement this and see.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: conversion error on index_from_pointer

Postby velocity » Fri Nov 26, 2021 8:44 pm

From SourceMod docs
When hooking an event, there are three modes to choose from:

Pre - Hook the event before it is fired.
Post - Hook the event after it is fired.
Post_NoCopy - Hook the event, but do not save any of its information (special optimization).
Hooking an event is usually done for one of the following goals. To get an idea of which mode to use, see the list below each goal:

Blocking the event (preventing it from being fired)
Always Pre
Rewriting the event (changing its parameters)
Always Pre
Acting upon the event (doing something once the event is completed)
Pre if your action must come before the mod's action.
Post if your action must come after the mod's action.
PostNoCopy if your action is Post and only requires the event name.


I think I'm just confused about Pre and Post in this case, as I thought the action had to come before the mod's action returning False/True, meaning I thought if the action already happened then in a post hook it is too late to change the state, but I'm guessing I completely misunderstood that :confused: - but no excuse for not trying and see of course.... :smile:
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Fri Nov 26, 2021 8:51 pm

Pre hooks are called before the original function is called, and post ones are called after but before the result is returned to the caller. At that point, you can no longer override a parameter, nor prevent what the original function does, but you can override the value it returned with your own which is what you want to do here. Which is much more efficient to filter a bool than re-implementing checks the function already did for you and I would actually be curious for you to run the same 15 seconds test you did and see the difference of how many calls you no longer have to process.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: conversion error on index_from_pointer

Postby L'In20Cible » Fri Nov 26, 2021 9:07 pm

By the way, while I think about it, something else you could do as well would be to replace your prototype:

Syntax: Select all

[DataType.POINTER, DataType.POINTER],
To:

Syntax: Select all

[DataType.ULONG, DataType.ULONG],

Which would remove the internal CPointer instantiation, as well as removing the need for you to resolve the address attribute because args[0] and args[1] would already be their address.

Return to “Plugin Development Support”

Who is online

Users browsing this forum: No registered users and 27 guests