Performance issue!!!

All other Source.Python topics and issues.
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Re: Performance issue!!!

Postby satoon101 » Sun Oct 14, 2018 1:57 am

I haven't read all of the replies, yet, but one thing really stood out in your tests reply. You stated you are using the ES Emulator to run the ES code. In case you are unaware, that literally wraps SourcePython functionality to emulate EventScripts. If you truly did a 1-to-1 conversion, which looking at the code clearly is not the case, there is no possible way it would be slower when using SP natively compared to using the emulator to call SP code. I hope to have time tomorrow to do an actual 1-to-1 conversion for you to test out.
Image
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: Performance issue!!!

Postby L'In20Cible » Sun Oct 14, 2018 2:05 am

velocity wrote:Now that we are on it:

If I remove all redundancy etc.. which I most certainly will, is this the best way of getting the nearest player (My main goal is actually to determine if the player is stuck inside another player and I actually did share my code there is no difference from my old ES plugin and SP only what SP force me to do differently), maybe the source engine has a function for it?
See, if you told us that from the beginning we would have been able to redirect you to engine_trace right away. Check out Entity.is_in_solid, a good example that test if an entity's physic box collide with another. Using the search feature would also have yielded some interesting topics such as: viewtopic.php?p=4084#p4084

velocity wrote:Another thing, this is the code I wrote to replace gamethread.delayedname. First of all, is this a valid replacement? Secondly, do I even need to make a gamethread, is it true that player attributes run on the main thread anyway? What doesn't?
I could not tell, to be honest. I personally only use threads for external requests.

EDIT:

I just wanted to share more inputs/thought process about how else you could have optimized your code. Let's say you changed what was previously suggested and this is now your code:

Syntax: Select all

player_instances = PlayerDictionary()

@EntityPreHook(EntityCondition.is_player, 'run_command')
def player_run_command(args):
index = player.index
player = player_instances[index]

if not player.dead or player.team_index < 2:
target = closeplayer(player, radius=200)


def closeplayer(player, radius):
for other in player_instances.values():
if not player.index == other.index and not other.dead:
dist = player.origin.get_distance(other.origin)
if dist <= radius:
return (dist, other)

return (10000, None)


The question you have to ask yourself when you are facing performance issues is, how can I improve my code so it has the same output with minimal work? Let's take in example:

Syntax: Select all

if not player.index == other.index and not other.dead:


Is retrieving the indexes and comparing them a necessity? The answer is no. You know your player object is contained into player_instances because this is where you retrieved it:

Syntax: Select all

player = player_instances[index]


So your condition could simply compare the object without any transformation using the is conditional statement:

Syntax: Select all

if player is not other and not other.dead:


Same result, no transformation; optimization.

Another question you have to ask yourself, especially when your code rely on constant heavy looping is, how could the amount of iterations be reduced? In your specific case, does iterating over all players a necessity? The answer is no. An optimal approach could be to keep a synced list of alive players and iterating over it instead. That way you are not only reducing the amount of iterations but, you are also removing the need to constantly test their dead status. For example, this could be achieved using game events:

Syntax: Select all

from events import Event    
from filters.players import PlayerIter
from players.helpers import index_from_userid

alive_players = PlayerDictionary()

def load():
for player in PlayerIter('alive'):
alive_players[player.index] = player

@Event('player_spawn'):
def player_spawn(game_event):
player = Player.from_userid(game_event.get_int('userid'))
if player.team_index <= 1:
return

alive_players[player.index] = player

@Event('player_death')
def player_death(game_event):
del alive_players[index_from_userid(game_event.get_int('userid'))]


Now we are sure that our alive_players dictionary always contain all the players that are alive, so we can simply change our implementation to:

Syntax: Select all

def closeplayer(player, radius):
origin = player.origin
for other in alive_players.values():
if player is other:
continue
dist = origin.get_distance(other.origin)
if dist <= radius:
return (dist, other)

return (10000, None)


But what does this means? This means:

  • We no longer have to retrieve and check other.dead.
  • We no longer have to iterate over dead players because they are being removed from our container as soon as they die (e.g. if there is 20 players on the server, but 10 of them are dead, we now only iterate 10 times instead of 20, etc.).

Same result, less iterations; optimization!

Basically, you always have to think about what your code is doing and how differently it could be implemented so that it doesn't do stuff that are not absolutely required at this exact moment.
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Re: Performance issue!!!

Postby satoon101 » Mon Oct 15, 2018 1:57 am

I apologize, I misread your post about using the emulator. So, you noticed this slowness while trying to run the ES code through the emulator. I will say, using SP natively will be faster than using the emulator. I'm fairly sure that the emulator was created so that people could still use old ES scripts without having to convert all of them to SP. However, he fact that it flows through 2 separate APIs in order to run is bound to slow it down. And again, your test 2 code is definitely written inefficiently, and does not replicate at all the code in test 1, which would cause slowness.

I unfortunately did not have any time today to test anything to sort out where slowness issues might be occurring. I think L'In20Cible has provided a number of proper optimizations. I still don't think a run_command hook is the proper way to do this. I would rather use a tick listener or at a Delay/Repeat. Then you can do something like this (untested):

Syntax: Select all

from filters.players import PlayerIter
from listeners.tick import Repeat

alive_players = PlayerIter('alive')

@Repeat
def _my_repeat():
closest_players = _get_closest_players()

def _get_closest_players():
player_origins = {player.index: player.origin for player in alive_players}
closest_distances = {}
closest_players = {}
indexes = list(player_origins)
for count, index1 in enumerate(indexes, start=1):
origin1 = player_origins[index1]
current_closest1 = closest_distances.get(index1)
for index2 in indexes[count:]:
origin2 = player_origins[index2]
distance = origin1.get_distance(origin2)
if current_closest1 is None or distance < current_closest:
current_closest1 = current_distances[index1] = distance
closest_players[index1] = index2
current_closest2 = closest_distances.get(index2)
if current_closest2 is None or distance < current_closest2:
current_distances2[index2] = distance
closest_players[index2] = index1
return {
index1: {
'closest_player_index': index2,
'distance': closest_distances[index1]
} for index1, index2 in closest_players.items()
}

_my_repeat.start(.01)
Image
User avatar
Ayuto
Project Leader
Posts: 2193
Joined: Sat Jul 07, 2012 8:17 am
Location: Germany

Re: Performance issue!!!

Postby Ayuto » Mon Oct 15, 2018 6:23 pm

I'm might be a little bit late to the party, but I still would like to drop my thoughts here.

First of all, I still think retrieving whether or not the player is dead is the bottleneck due to the slow implementation of looking up properties dynamically. I just did a quick test and was able to prove my assumption:

Syntax: Select all

from timeit import Timer

ITERATIONS = 100000

print(Timer(
"""
player.dead
""",
"""
from players.entity import Player

player = Player(1)
""").timeit(ITERATIONS))

print(Timer(
"""
player.get_datamap_property_bool('pl.deadflag')
""",
"""
from players.entity import Player

player = Player(1)
""").timeit(ITERATIONS))
Output:

Code: Select all

1.0602915173727183
0.0478759625112275


A few more points:
  1. EventScripts gamethread module never created real threads unlike Source.Python's GameThread class. The proper equivalent is Delay/Repeat.
  2. The ES Emulator is completely written with Source.Python's API. So, there is an extra layer that will slow down ES addons. Moreover, it was quickly written, because I wanted it to work -- not to work fast. So, there is definitely much space for improvements.
  3. If you want to compare distances (not to get the actual distance), I would recommend to use get_distance_sqr. That function skips calculating the square root (don't forget to sqare your radius).
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: Performance issue!!!

Postby velocity » Tue Oct 16, 2018 1:46 am

Thank you!!

Do I need to change my code or this will be in a pull request, making player.dead equivalent to player.get_datamap_property_bool('pl.deadflag') ?
User avatar
satoon101
Project Leader
Posts: 2697
Joined: Sat Jul 07, 2012 1:59 am

Re: Performance issue!!!

Postby satoon101 » Tue Oct 16, 2018 2:19 am

We moved most (all?) of the CBaseEntity properties to C++. Perhaps we should do the same with some of the common CBasePlayer ones, which are found in all games we support.
Image
User avatar
Ayuto
Project Leader
Posts: 2193
Joined: Sat Jul 07, 2012 8:17 am
Location: Germany

Re: Performance issue!!!

Postby Ayuto » Tue Oct 16, 2018 5:22 am

I didn't move that property, because it's not shared across all entities. You can use this as a workaround for now.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: Performance issue!!!

Postby velocity » Tue Oct 16, 2018 2:31 pm

Another thing I had my doubts about, is it worth making a gamethread with a repeater inside (performance wise), or just having a repeater? Particularly for use cases like the player class.
User avatar
Ayuto
Project Leader
Posts: 2193
Joined: Sat Jul 07, 2012 8:17 am
Location: Germany

Re: Performance issue!!!

Postby Ayuto » Tue Oct 16, 2018 7:13 pm

I can't really tell, but I don't think so.
User avatar
velocity
Senior Member
Posts: 220
Joined: Sat May 10, 2014 6:17 pm

Re: Performance issue!!!

Postby velocity » Tue Oct 16, 2018 8:52 pm

Is it something you could test? At this point, I don't know how to measure that.
User avatar
L'In20Cible
Project Leader
Posts: 1533
Joined: Sat Jul 14, 2012 9:29 pm
Location: Québec

Re: Performance issue!!!

Postby L'In20Cible » Wed Oct 17, 2018 4:16 am

Ayuto wrote:Output:

Code: Select all

1.0602915173727183
0.0478759625112275

21 times slower is a bit terrifying. :eek:

satoon101 wrote:We moved most (all?) of the CBaseEntity properties to C++. Perhaps we should do the same with some of the common CBasePlayer ones, which are found in all games we support.


Ayuto wrote:I didn't move that property, because it's not shared across all entities.


One thing we could consider; is to wrap PlayerInfo.is_dead to point to that property instead and just forward Player.dead to it. This would also fix the false results returned for games other than HL2:DM.

Return to “General Discussion”

Who is online

Users browsing this forum: No registered users and 21 guests