[Kstars-devel] Fwd: Re: Speeding things up

[Khudyakov Alexey]

On Четверг 11 июня 2009 22:24:27 James Bowlin wrote:
> On Thu June 11 2009, Khudyakov Alexey wrote:
> > I think it's better to try to optimize constructors and all in order
> > to bring perfomance on new/delete version to memcpy version. This
> > remove possibility of rellay unpleasant surprises. And if it's proven
> > to be impossible fall back to memcpy.
>
> I wouldn't mind losing 10% performance here if it makes you more happy
> with how the code looks.  My main concern is the speed of filling the
> StarObjects with data which occurs in StarBlockList::fillToMag() via
> the call to StarObject::init( &stardata ).
>
> This is the trick that is used to efficiently "recycle" StarObjects by
> totally avoiding creation and deletion.  IMO, it is more honest (clear)
> to use memcpy and malloc when the space for the StarObjects is first
> created rather than using a constructor with phony data.
>
> The call to StarObject::init() above will fill an "empty" StarObject
> with actual data before it is used for the first time.  It will also
> "overwrite" a StarObject that is no longer going to be used with data
> for a "new" StarObject.  This is where we get the speed and this code
> gets exercised much more often than the creation of a new StarBlock.
>
> In fact, I don't think we ever free up StarBlocks before the program
> terminates.  I think it would be a good idea to slowly free up unused
> StarBlocks but AFAIK this has not been implemented yet.  The memcpy
> + later init() scheme was designed to make all of this as fast as
> possible.  In the original design (on paper) we allocated the memory
> for an entire StarBlock with one malloc().  For the time being we did a
> bunch of little StarObject sized mallocs because I thought this would
> be easier on the "buddy list" by not asking for a lot of large chunks
> of memory.
>
> Since we never got to freeing up StarBlocks, we never even tried the
> more efficient malloc scheme because were we pressed on other issues
> and because the code never got exercised much.  This makes me realize
> that I do object to trying to get rid of the current malloc().  I
> suppose that placement new could be used instead of the memcpy if it
> doesn't significantly hurt performance but that would imply
> a "placement delete" which would not be used if we malloc an entire
> StarBlock at once.
>
> IMO, it is a little premature to switch over to new/delete now merely to
> avoid the theoretical "possibility of really unpleasant surprises".

But than. It's easy. Code could get faster and safer at the same time. 
And all memcpy/placement new stuff could be just thrown away.

The idea is very simple. There is no need to pass StarObject in addStar 
function and than copy it. Just pass StarData/DeepStarData and call init on 
already allocated StarObject.

So everything could be simpler. Just allocate array of empty StarObjects at 
StarBlock creation. This is what's done at the moment. Some pseudocode

class StarBlock
{
public:
  bool addStar(const StarData& d);
private:
  QVector<StarObject> stars;
};

bool StarBloc::addStar(const StarData& data)
{
  // Omitting all checks
  stars[nStars].init(data);
  nStars++;
}

Only important thing is that it should be assured that "init" clears star 
completely.

Thank you James!

--
  Khudyakov Alexey