dsjoerg
commented
11 years ago One wrinkle to consider when architecting this.
Consider a plugin A that reads through the entire replay and then sets some variables on various objects. And then a plugin B that also reads through the entire replay and uses the variables set by plugin A.
Rewriting those plugins to use the event loop may be impossible, in particular if the values that plugin B needs are based on values that are only available when plugin A is done consuming all the events of the replay.
It's effectively a look-forward dependency.
You could response to this challenge in one of three ways:
- Punt! Plugins that work as described above could continue to just process all the events themselves
- During plugin registration, there could be an "event loop level". Everything on level 0 gets run together, and then if there's a level 1, then everything on level 1 gets run together, and so on.
- When one plugin depends on another, its inbound events might queue up until its dependent plugin says that it is done processing. This might be a memory hog though.
This is not a hypothetical problem; I believe that BaseTracker and the upcoming ProtossMacroTracker have this A/B relationship. BaseTracker uses last_activity, which is not known until the replay is done processing. But ProtossMacroTracker would be consuming events in order.
When you say the new approach would be much faster, it would be helpful to put that in context by measuring what % of total replay processing time is taken up by plugins. I don't have good benchmarks for that, but before pouring a bunch of time into this (at least for ggtracker's sake), it would be important to know the theoretical max improvement in processing time. We're not using pypy yet on the server, and that would be a ~2x speedup for maybe 1 day's work messing with sysadmin, because python processing is only about 35% of the overall ggtracker processing latency.
GraylinKim
Processing right now performed by registering a series of plugin's that run over the replay sequentially. In an event loop, processing would happen by registering callbacks to different event types and looping through the callbacks on each event before moving to the next.
I can see a few reasons why this is better:
load_contextmethods on events could be factored out into a initially enabled plugin. This would create a stronger divide between the code that loads and processes the replays.So we're doing it. People that don't like it can pipe the replay through the existing plugins themselves.