Performance issue!!!
Re: Performance issue!!!
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.
- L'In20Cible
- Project Leader
- Posts: 1534
- Joined: Sat Jul 14, 2012 9:29 pm
- Location: Québec
Re: Performance issue!!!
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#p4084velocity 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?
I could not tell, to be honest. I personally only use threads for external requests.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?
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.
Re: Performance issue!!!
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):
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)
Re: Performance issue!!!
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:
Output:
A few more points:
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))
Code: Select all
1.0602915173727183
0.0478759625112275
A few more points:
- EventScripts gamethread module never created real threads unlike Source.Python's GameThread class. The proper equivalent is Delay/Repeat.
- 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.
- 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).
Re: Performance issue!!!
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') ?
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') ?
Re: Performance issue!!!
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.
Re: Performance issue!!!
I didn't move that property, because it's not shared across all entities. You can use this as a workaround for now.
Re: Performance issue!!!
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.
Re: Performance issue!!!
I can't really tell, but I don't think so.
Re: Performance issue!!!
Is it something you could test? At this point, I don't know how to measure that.
- L'In20Cible
- Project Leader
- Posts: 1534
- Joined: Sat Jul 14, 2012 9:29 pm
- Location: Québec
Re: Performance issue!!!
Ayuto wrote:Output:Code: Select all
1.0602915173727183
0.0478759625112275
21 times slower is a bit terrifying.
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 23 guests