Re: [Grammatica-users] Re: Grammatica-users Digest, Vol 15, Issue 3

[Per Cederberg]

Ah, of course! I noted that slight change but didn't think more
of it (actually considered a typo... :-)

Now then, once understanding the patch I must point out an issue
with it. Or rather a bug in the Sequence class that this patch
exposes. The thing is that Sequence overrides equals() but not
hashCode()... So the add operation in the Set (that relies on the
hash code to find a hit) will not be correct in all situations as
the default hash code is based on the object instance location in
memory (ie a pointer).

The end result will be that some look-ahead sequence might be
added twice to the LookAheadSet. Maybe not such a big deal that it
matters, but it may cause some duplicated output when showing the
debug printout for a grammar.

Now, I guess one could try to simply remove the duplicate check
in LookAheadSet (keeping the ArrayList implementation) and that
should give the same results. Otherwise the new solution might
become slow again if a correct (but possibly slow) implementation
of hashCode() is ever added to the Sequence class.

/Per

On tue, 2005-05-31 at 21:20 +0200, Steve Davis wrote:
> Hi Per,
> 
> Indeed, the iterators were only necessary to cope with the Set interface 
> (i.e. no .get(int)).
> 
> Profiling the code showed that Grammatica was spending all its time in 
> prepare()... and specifically the call to add(Sequence) from the 
> addAll() method of LookAheadSet. In fact over 95% of its time...
> 
> So in summary, the single line improvement that made all the difference 
> was using the platform to determine the uniqueness of the Sequence 
> rather than doing a manual check.
> 
> i.e.
>     private void add(Sequence seq) {
>         if (seq.length() > maxLength) {
>             seq = new Sequence(maxLength, seq);
>         }
>         elements.add(seq); // It's a Set! No duplicates!
>     }
> 
> ...as compared to...
> 
>     private void add(Sequence seq) {
>         if (seq.length() > maxLength) {
>             seq = new Sequence(maxLength, seq);
>         }
>         if (!contains(seq)) {
>             elements.add(seq); // A List can contain repeated values, so 
> we must check.
>         }
>     }
> 
> The grammar file is over 1000 lines and very much a development version, 
> but any feedback you have time to give would be more than appreciated, 
> as it's my first attempt at any sort of EBNF. I'll mail it to you 
> separately.
> 
> Regards,
> Steve