Re: [Civlog-list] CivLog-0.6.1

[Raimar F. <hawk@B205a.WH8.TU-Dresden.De>]

On Tue, Oct 03, 2000 at 12:54:52PM +0100, Michael Grundel wrote:
> Raimar wrote:
> > On Mon, Oct 02, 2000 at 11:21:21PM +0200, Ulrich Kuhn wrote:
> > > yOn Mon, 2 Oct 2000, Michael Grundel wrote:
> > > 
> > > > > A few littleties:
> > > > > For memory's sake and as it is my job :-) I recommend the following patch
> > > > > for DataTypeInfoForOneTurn.java:
> > > > >
> > > > > 27,28c27,28
> > > > > <         data = new int[FreecivConstants.MAX_NUM_PLAYERS];
> > > > > <         defined = new boolean[FreecivConstants.MAX_NUM_PLAYERS];
> > > > > ---
> > > > > >         data = new int[4];
> > > > > >         defined = new boolean[4];
> > > > > 44a45
> > > > > >         if (player >= data.length) grow(Math.max(data.length*2, player));
> > > >                                                  ^^^ ^^^^^^^^^^^^^
> > > > We should be careful not to make it larger than FreecivConstants.MAX_NUM_PLAYERS 
> > > > by doubling.
> > > > Could this happen or did I miss something?
> > 
> > As Ulrich mentions below currently this can not happen. However there is no
> > harm if data is larger than FreecivConstants.MAX_NUM_PLAYERS.
> 
> I thought the whole point of this was to save space? Would a doubling to 64 
> not waste a lot of space?
> 
> > DataTypeInfoForOneTurn doesn't have enough information to check for the
> > validity of a given playernumber. However it can test if the value was set
> > previously.
> > 
> > > Since MAX_NUM_PLAYERS is 32 this can not happen: 4 -> 8 -> 16 -> 32
> > > So the highest player number will be 31 and that fits in an array with 32
> > > places.
> 
> IMHO this is no valid argument. In fact it only works if MAX_NUM_PLAYERS is 
> 2^x. I think we should not make such assumptions. It depends on freeciv and 
> could well be increased in the future (to a non 2^x value). 
> Lets say it is increased to 40 in freeciv, we would get: 4 -> 8 -> 16 -> 32 
> -> 64.
> 
> Since the whole purpose of this is to save some (small amount) of memory, 
> I believe this should be fixed (if it is done at all).
> 
> A possible solution:
> if (player >= data.length) grow(Math.min(Math.max(data.length*2, player), 
> FreecivConstants.MAX_NUM_PLAYERS);

If we grow I would second this line. However I would like to see some
measurements of the performance of the grow() method or the arraycopy() call.

	Raimar

-- 
 "Like the ad says, at 300 dpi you can tell she's wearing a
  swimsuit. At 600 dpi you can tell it's wet. At 1200 dpi you
  can tell it's painted on. I suppose at 2400 dpi you can tell
  if the paint is giving her a rash."
    -- Joshua R. Poulson