[Kstars-devel] Great minds think alike: AuxInfo Hash

[James Bowlin]

On Thu June 5 2008, Akarsh Simha wrote:
> I don't understand what you mean here, epsecially about stars that
> are not in the index. Which index are we talking about?

If every star had an HD index number then we could use that for a unique
identifier.  Since (as I understand) some stars don't have an HD index
number, we can put our own negative numbers in the HDIndex field for 
those stars so we can use that field as a unique ID for every star.
The number is unique for every star.  If it is positive, it is the 
actual HD index.  If it is negative then it is a number we created in
order to get a unique ID for every star.

The only other alternative I see is adding an addition int32 to create
our own unique ID for every star.  My scheme above means we don't have
to add another field to StarData.

> There is one more issue - the links / log apply to not only stars,
> but all kinds of objects - stars, comets, asteroids, planets, DSOs,
> and sattelites!
>
> I think the best way to handle this is to have a UID for every object
> in the sky. For stars, this could be the HD number in the files on
> the disk. If not all stars have HD numbers, we could extend it (int32
> can store far more than HD numbers) and have a flag telling us
> whether the star's UID is a HD number or something that we cooked up.
> Once we read a star's HD number, we could prefix it with some sort of
> identifier that tells you that this SkyObject is a StarObject. This
> will help distinguish HD 001000 from NGC 1000 and IC 1000.

Yes, I thought of this too.  The thing is that for most objects, we 
don't need to go to the trouble of going through a hash if we just 
create the AuxInfo objects as needed as I described below.


> > Even if we want to disable the AuxInfo fields for unnamed stars for
> > now, I think we may need to do something in order to prevent
> > segfaults and memory leaks when we dynamically load stars.  I think
> > either unnamed stars will have to no longer subclass from SkyObject
> > or we are going to have to change the SkyObject data structure.
>
> Why will the current setup cause segfaults when we dynamically load
> stars?

Good question.  Maybe it wouldn't cause a segfault.  I was concerned 
because I thought you had reported getting segfaults issues with 
certain data fields in StarObjects.

> Currently, I do not encounter a segfault at all. Things seem to go on
> very smoothly, but the logs aren't saved, as used to be the case even
> earlier.

I misunderstood.

> I would prefer to go with the latter of the two options and
> dissociate the user data and the info from the SkyObject.

Good.  I think we all agree.


> I find both the idea of putting a pointer, and the idea of having a
> hash very good. I wouldn't want to inherit UnnamedStar from SkyPoint,
> though.

Agreed.

> I somehow prefer the hash solution, because it keeps the data that we
> will keep together on the disk together even during runtime, but the
> problem of generating UIDs for all the objects will need to be
> tackled. The good thing is that we need not have the bits for
> identifying object type uniquely on the disk, but we can assign those
> during runtime.

I think we need the hash for the dynamically loaded stars but I don't 
see any benefit to using the hash for SkyObjects that will always be
in RAM.  We could certainly implement a more generalized hash solution
for every object, but unless there is a benefit from doing that, I see
it as extra work and complexity (and RAM) that doesn't gain us anything.

> Yep. I guess it can be done in a day.

Jason may already be working on changing SkyObject so please coordinate 
with him.  

> I need the HD numbers for our existing catalog beforehand, though.

Perhaps you have a slightly different vision of the way it would work, 
but the way I see it the unique ID we create from the HD index is only
needed to keep an AuxInfo tied to a particular star while KStars is 
running.  Therefore you could just use negative numbers for every star
(for now) and then when the HD index numbers arrive we can change the 
field to the combined HD index / negative number that I suggest above.  
I don't see where these numbers get saved to disk (except in the binary
star data file) so even if they all changed every time KStars started,
there wouldn't be a problem.

On the other hand, I see no problem with delaying the implementation of
the AuxInfo hash.  As long as we aren't doing a memcpy of structures we
are going to write to, I'm happy.

> > This also creates the hooks needed for implementing a hash of
> > AuxInfo objects for unnamed stars.  When an AuxInfo object is
> > required for an unnamed star, instead of simply creating a new
> > AuxInfo object, we first check the hash to see if an AuxInfo object
> > for this star already exists.  If so, we set the AuxInfo pointer to
> > the location of the existing AuxInfo object.  If not, we create a
> > new AuxInfo object, add it to the hash and set the pointer.
>
> Ahh... this looks perfect.

Okay.  Maybe we are in full agreement then.

> > I think we may have to implement the AuxInfo class and pointer now
> > in order to prevent problems with dynamically loading stars even if
> > we decide to hold off on implementing the AuxInfo hash.
>
> I still don't understand how it will affect dynamic loading.

It appears I overstated the case for the problems the current structure
would cause.  I must have misunderstood when I thought you had 
previously reported segfault problems.

If we do nothing it still seems to me there still might be problems 
because the QStrings and QStringLists that we memcpy will have internal 
pointers and references that are now all pointing / referring to the 
same memory.  It is possible that the internal structure of QString and 
QStringList is robust enough to withstand this kind of abuse but I 
would hate to assume it is all just going to work complaint free.  Even 
if it doesn't fail catastrophically, it certainly won't do what we want 
it to do.  There would be just one userLog, etc for all of the dynamic 
stars.

Using a pointer to AuxInfo and creating AuxInfo as needed gets around 
this first problem (as well as reducing overall memory use).  The use 
of the hash for the dynamic stars then solves the two remaining 
problems with AuxInfo for dyanamic stars.  First, it allows the 
redundant stars (due to proper motion) to point to the same AuxInfo, 
and it also gives us persistence even when stars get swapped out and 
swapped back in again.   It doesn't (in itself) give us persistence 
across restarts of Kstars, but it is not designed to do that.  Perhaps 
that persistence is provided elsewhere, but I think it is an 
independent problem.


-- 
Peace, James