69271 - pldhash.[ch] (jsdhash.[ch]) spruce-up catchall bug

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[Brendan Eich [:brendan]]

- js_HashString in jsstr.c should not ape java.lang.String's bogo-sampling
"long" strings.
- ditto JS_HashString.
- ditto JS_DHashStringKey
- gordon reminded me of a silly misfeature of js/pldhash.c when growing a table
where half the capacity are entries, a quarter are removed sentinels, the table
can grow and then (if there's another removal) shrink immediately.

Patch coming up.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: js/src-relative patches (xpcom/ds/pldhash.[ch] come from sed -f plify_jsdhash.sed)

Comment 2

[Brendan Eich [:brendan]]

My third 0.8.1 bug to fix.  I'd better get that follow-up patch hacked up.

/be

Comment 3

[gordon]

Brendan, we on the new cache team are eager for your changes. :-)

Comment 4

[Brendan Eich [:brendan]]

Attachment: final patch for 0.8.1, I hope -- review me please!

Comment 5

[Brendan Eich [:brendan]]

Attachment: final patch for 0.8.1, I hope -- review me please!

Comment 6

[Brendan Eich [:brendan]]

I can't believe Mozilla took a double-click on the attachments page submit
button to mean attach the same file twice.  Cc'ing pollmann -- eric, what's the
scoop?  Known bug?

/be

Comment 7

[Mike Shaver (:shaver emeritus)]

Nice comment! sr=shaver

Comment 8

[Chris Waterson]

Maybe note that you are avoiding a branch with the bitwise-or (so it's not
``fixed'' later). r=waterson

Comment 9

[Brendan Eich [:brendan]]

Attachment: better fix (shrinks after removing during Enumerate), trying for 0.8.1

Comment 10

[Chris Waterson]

Why would you call ChangeTableSize() with a zero value for deltaLog2?

Comment 11

[Chris Waterson]

Talked with brendan, and he explained why this is necessary (due to collisions,
``removed'' entries still take up space sometimes). r=waterson (or sr=, if you
need it)

Comment 12

[Brendan Eich [:brendan]]

waterson's fine question caused me to rename ChangeTableSize ChangeTable, since
it doesn't necessarily change the table size.  But it must change the table's
entry storage, even if only compressing to eliminated removed tombstone entries.
I also commented on how its loop moves only the live entries from oldEntryStore
to newEntryStore, leaving removed (and skipEntry) behind.

/be

Comment 13

[chris hofmann]

ready to go when leaf-er-oni opens the tree?

Comment 14

[Brendan Eich [:brendan]]

Actually, I checked in before the deadline, got in line at the metering light
and everything.

/be