Re: [aspell-devel] Aspell core dump

[Kevin Atkinson]

On Fri, 17 Dec 2004, Gary Setter wrote:

> <snip>
> > The void pointer array in WordEntry is simply a generic place
> to stick
> > some data.   It is a rather common practice so I don't see how
> that is
> > hard to understand.  The alternative would be to make WordEntry
> a virtual
> > base class.  This will involve unnecessary allocation of
> dynamic memory and
> > functions calls.  The way I decided WordEntry was that no extra
> dynamic
> > memory needs to be allocated, and in the common case of only
> one word no
> > functions calls are needed.  I am unlikely to change it.
> >
> > The "(StrVector *)(s - sizeof(StrVector) - 2)" is there because
> it
> > simplified storing the data in the hash table.  I am open to
> patches that
> > avoid it with out adding significant performance penalty or
> adding a lot
> > of extra code.  Of course I expect the changes to self
> contained within
> > the file writable.cpp.
> >
> > Don't even think about changing readonly_ws.cpp the data there
> is very
> > carefully laid out to avoid wasting a lot of space with extra
> pointers.
> >
> Hi Kevin,
>
> I understand your point of view. I started out working in
> assembly on a machine with inadequate memory and I've had to
> crunch code.
>
> It's been some years since I've had to put efficiency before
> maintainability. I thought that the software profession in
> general was moving the in maintainability direction. This may be
> one area where efficiency trumps maintainability.

Well in readonly_ws.cpp the space savings is significant.  In writable.cpp
I don't care as much as the number of words is is order of magnitude
smaller and the primary focus is to make adding new words (and possible
deleting exiting ones) easy and effect.  However I will likely reject
something that increases space by too much.

> Even if you choose not to accept it, I plan to submit a patch to
> writable.cpp defines structs for how words and sounds like data
> is stored in the WordLookup and SoundslikeLookup. If I never try,
> I'll never know if making the change would be worth while.

I will have to see a patch before I can give you an answer.

> Puttering around with the code made me ask why you store the
> lengths with all word and soundslike data. It does not seem to be
> used except as a check against the null terminated length of
> strings. Is there a need with some languages or character sets to
> store nulls within words. Is there a chance of corruption and you
> need to redundancy?

The length is stored as an optimization to avoid having to recompute it
each time, it is only a byte to the size/speed tradeoff is worth it.  It
is also null terminated for simplicity things as most of Aspell expects
strings to be null terminated.  Also, I have found there is a benefit to
storing both as there are cases when using the size is better and cases
when looking for the null character is better.

> It would reduce the complexity if the soundslike data was not
> stored with string length. How do you feel about that change?

If you are talking about the layout in writable.cpp than fine and there is
a chance I will accept it.  I will have to see the patch before I give you
an answer.