[Freecol-developers] getGame() + getSpecification() calls

[Enrico Weigelt, metux IT consult]

Hi folks,

we've got *a lot* of objects holding a reference to Game objects
and lots of places retrieving it via getGame() calls, or getting
specification object Game::getSpecification().

Is there any usecase for having multiple Game objects within one
process ? Otherwise we could make it a singleton, for efficiency.
(we could even make its contents, eg specification singleton)

--mtx

PS: merry christmas

[Michael T. Pope]

On Sun, 25 Dec 2016 18:31:19 +0100
"Enrico Weigelt, metux IT consult" <***@gr13.net> wrote:
> we've got *a lot* of objects holding a reference to Game objects
> and lots of places retrieving it via getGame() calls, or getting
> specification object Game::getSpecification().
>
> Is there any usecase for having multiple Game objects within one
> process ? Otherwise we could make it a singleton, for efficiency.
> (we could even make its contents, eg specification singleton)

There is usually only zero, one, or two game objects (client + server).
However there are some corner cases, and handling the places where the
client and server threads transition from having no game to having a new
game and communicating their state to the player threads, and then
subsequently clearing back to no game... well it is better than it was a
few weeks ago because I have been busy in that area but it is still
non-trivial. Beware also that in the longer term we hope to change things
so that the game currently shared by the server and the AI clients can be
distinct.

As usual I have to ask what problem you are trying to solve? It sounds
like you are worried that the getGame calls are inefficient. Please do
some profiling. You will see that stuff like this is down in the noise.
Or indeed, imagine that you went ahead and turned all the game objects
into suitable singletons and accessed them that way, *and* measured the
difference --- let me speculate the gain would be *tiny* if anything as a
lot of those calls will be getting auto-inlined anyway.

I am sorry I keep arguing with you, I really appreciate that you want to
improve the FreeCol codebase, but we need to put our effort into stuff
that is *broken*. FreeCol is without a release for too long, there is a
serious bug blocking release, I think it can be fixed by finishing a long
planned but eternally postponed low-level c-s rewrite, which is mostly
done, but some hard bits remain. Destabilizing the code with changes that
do not actually fix bugs or improve performance is not a good idea
right now, at least until we can get 0.11.7 released.

Cheers,
Mike Pope

[Enrico Weigelt, metux IT consult]

On 25.12.2016 22:58, Michael T. Pope wrote:

Hi,

> There is usually only zero, one, or two game objects (client + server).

Are client and server both running in the same process ?
In which ways do these two instances differ ?

(code pointers appreciated :))

Another open question: what about the Specification class ? Can there
be multiple instances at the same time ?

> However there are some corner cases, and handling the places where the
> client and server threads transition from having no game to having a new
> game and communicating their state to the player threads, and then
> subsequently clearing back to no game... well it is better than it was a
> few weeks ago because I have been busy in that area but it is still
> non-trivial. Beware also that in the longer term we hope to change things
> so that the game currently shared by the server and the AI clients can be
> distinct.

Ah, AI players are basicly just usual players ? Could they, in theory,
run in a different process, or on a different machine ?

> As usual I have to ask what problem you are trying to solve? It sounds
> like you are worried that the getGame calls are inefficient.

Well, they probably do some overhead, as they seem to be called quite
often, but I don't know much actually take.

I'll yet have to find out, whether
a) these functions get inlined
b) they get jit-compiled at all (according to official docs, a method
has to be called 1k times before jit gets active)
c) references to long-lived objects or final refs also get inlined

I'm not so confident w/ java profiling, so any tips appreciated.

> Or indeed, imagine that you went ahead and turned all the game objects
> into suitable singletons and accessed them that way, *and* measured the
> difference --- let me speculate the gain would be *tiny* if anything as a
> lot of those calls will be getting auto-inlined anyway.

Does the inlining also happen on bytecode level - before jit gets active
? The reason for my question: according to the jre docs,
methods have to be called 1k times before jit starts.

> I am sorry I keep arguing with you, I really appreciate that you want to
> improve the FreeCol codebase, but we need to put our effort into stuff
> that is *broken*.

Okay, which ones do you suggest ?


--mtx

[w***@genial.ms]

Hi,

> Gesendet: Montag, 26. Dezember 2016 um 20:44 Uhr
> Von: "Enrico Weigelt, metux IT consult" <***@gr13.net>
> An: freecol-***@lists.sourceforge.net
> Betreff: Re: [Freecol-developers] getGame() + getSpecification() calls
>
> I'm not so confident w/ java profiling, so any tips appreciated.
>

>From what I remember when doing it some time ago, the client-server communication
looks as if it was a hotspot, but I think thats wrong as its counting
waits on sockets as if they used processor time. The xml parsing also
contributed some.
The real hotspot is MapViewer.displayMap, as there are huge nested loops,
but I already improved it some. It ends up inside simple stuff like Map and,
for example, Map.getTile gets called a huge number of times, such that a small
improvement there could be huge.
One really bad spot called from the map display is ImageLibrary.getStringImage
and if you could reimplement the outlined text drawing in some clever way,
without the very slow getRBG and setRGB calls, it would be a nice improvement.

> > Or indeed, imagine that you went ahead and turned all the game objects
> > into suitable singletons and accessed them that way, *and* measured the
> > difference --- let me speculate the gain would be *tiny* if anything as a
> > lot of those calls will be getting auto-inlined anyway.
>
> Does the inlining also happen on bytecode level - before jit gets active
> ? The reason for my question: according to the jre docs,
> methods have to be called 1k times before jit starts.

I think that wont matter for performance, but I have a dislike for singletons,
as they hide dependencies and encourage them. If you insist on trying to improve
that, you should instead try to have less classes need access to those often
shared references.

> > I am sorry I keep arguing with you, I really appreciate that you want to
> > improve the FreeCol codebase, but we need to put our effort into stuff
> > that is *broken*.
>
> Okay, which ones do you suggest ?
>

If you reimplemented GUI.createMiniMapThumbNail it would be appreciated, as
it is bad reuse of the gui class drawing the ingame minimap and
IIRC it is still bugged in that the map editor does not save a thumbnail image.


Greetings,

wintertime

[Michael T. Pope]

On Mon, 26 Dec 2016 20:44:40 +0100
"Enrico Weigelt, metux IT consult" <***@gr13.net> wrote:
> On 25.12.2016 22:58, Michael T. Pope wrote:
> > There is usually only zero, one, or two game objects (client + server).
>
> Are client and server both running in the same process ?

Yes in single player, no in multiplayer. For game play purposes there is
no need for them to be in the same process in single player either. OTOH
for development purposes, debug mode is pretty much useless without access
to the server in the client, and FreeCol is *not* at a stage where we can
do without debug mode yet.

> In which ways do these two instances differ ?

None AFAICT.

> Another open question: what about the Specification class ? Can there
> be multiple instances at the same time ?

Specification is a field in Game. It should follow the same pattern as
Game. Sometimes the Specification field in the client or server Game is
null, sometimes it is changed, and then messaging occurs to synchronize it
with the other party and the other clients. IIRC this can only happen
independently of Game at the pre-game stage (see Game + MapGenerator
Options). Once the Game starts, Specification should not change.

> Ah, AI players are basicly just usual players ?

Not yet. AIs still essentially live in the server, and exploit the extra
information available to them mercilessly. This has been one of my long
term projects --- slowly decouple the AIs to the point that they *could*
be ordinary players. The major obstacle that remains though is that the
AI is just not smart enough, and still relies on blatant cheating.

> Could they, in theory,
> run in a different process, or on a different machine ?

Not yet, but I have been pushing them in that direction, step by step.
The comms portion of this should all "just work". The next *big* step is
to stop the AIs from directly accessing the server Map.

> > As usual I have to ask what problem you are trying to solve? It sounds
> > like you are worried that the getGame calls are inefficient.
>
> Well, they probably do some overhead, as they seem to be called quite
> often, but I don't know much actually take.
>
> I'll yet have to find out, whether
> a) these functions get inlined
> b) they get jit-compiled at all (according to official docs, a method
> has to be called 1k times before jit gets active)
> c) references to long-lived objects or final refs also get inlined

I think you will find the answer is both Yes and No to all of the above:-).
It is complex and depends on a whole range of compiler, run-time and
system parameters. Consider also, if you find out what *your* machine
does, should we make optimization decisions based on just that one
configuration? Remember that FreeCol is intended to run on three
reasonably distinct operating systems, on <whatever> hardware (within
reason), and is sometimes compiled on site by our users so we can not just
ship .jar files and think we know exactly how they will behave.

> I'm not so confident w/ java profiling,

Neither am I, but I have used various GUI tools and JRE hacks from time to
time when we had a serious bug that was causing a hang or a massive slow
down. I do not recall much detail, but it was not difficult to work out.
Here is a chance for you to learn:-).

> > we need to put our effort into stuff that is *broken*.
>
> Okay, which ones do you suggest ?

I can not improve on wintertime's suggestions.

- If you want to work on optimization the display code has *always* been
the major bottleneck every time I checked (modulo the outright bugs and
hangs). The next big category is almost always due to the client-server
I/O with the hottest routines being low in the Java libraries where the
bytes are being shovelled or where we are waiting for input --- there is
not much we can do about that (not our code), but what we could do is
try to read/write less data. There has not been much effort in that
direction, because we struggle just to get to correctness.

- Yes, GUI.createMiniMapThumbNail is a mess. I think it should be
completely rewritten in the MapEditorController (only place it is
called) as just a simple image creation routine. There is no reason for
it to mess with MiniMaps. This problem is blocking a newly contributed
map from being added, so it is important work.

Cheers,
Mike Pope