73540 - [JS|PL]DHashTable should clear live entries when finalizing

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Chris Waterson]

The JSDHashTable and PLDHashTable implementations should clear the live entries
in [JS|PL]_DHashTableFinish().

Comment 1

[Chris Waterson]

Attachment: make JS_DHashTableFinish() clear live entries

Comment 2

[Brendan Eich [:brendan]]

waterson observed that Finish didn't, and I said "oh! that's a bug". Thanks to
waterson for filing and patching it.

Nit alert, I still feel parental about this code:

- no tripled newlines (two empty lines) in my code.
- proper sentence style (capitalized first word, full stop at end) in comments
that stand on their own line(s).

More significant, how about METER'ing removeHits++ for better stats?

Finally, should the induction variable i be eliminated, along with capacity, by
setting char *entryLimit; to entryAddr + JS_BIT(table->sizeLog2) * entrySize and
bounding entryAddr < entryLimit in the loop condition?  Same goes for Enumerate
(I have no excuse :-), on which this code was based.

/be

Comment 3

[Chris Waterson]

Attachment: updated, per feedback

Comment 4

[Chris Waterson]

I went ahead and modified JS_DHashTableEnumerate(), too. It's a wash, because
you trade `i' for `entryAddr'. If you don't like it, I'll revert.

Comment 5

[Brendan Eich [:brendan]]

I like it (I wouldn't have block-scoped entry in Finish, but I'm such a luddite)
and you did eliminate an induction variable in Enumerate, no wash -- it's true
you still need capacity after the loop, so entryLimit adds to register pressure,
but you don't need a = 0 before the loop and a unconditional i++ at the bottom
of the loop.

r/sr=[e-mail], anyone on the cc: list care to r=?

/be

Comment 6

[Mike Shaver (:shaver emeritus)]

With no unconditional i++ at the end of the loop, how could I say no?

sr=shaver

Comment 7

[Chris Waterson]

Fix checked in.

Comment 8

[Phil Schwartau]

Marking Verified -