610022 - WeakRefList should have a method to prune collected items

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, enhancement)

Description

[Steven Johnson]

There are lots of ad-hoc bits of code in Flash/AIR that prunes "dead" entries from lists of WeakRefs; we should add such a method to WeakRefList directly.

Comment 1

[Steven Johnson]

Attachment: Patch

Comment 2

[Steven Johnson]

Attachment: Patch v2

Once again, in slightly more efficient form

Comment 3

[Steven Johnson]

TR 5521:bd91f230f82e

Comment 4

[Steven Johnson]

Bah, existing code is clearly bogus; reopening (and will submit with a testcase this time, bad programmer, no cookie)

Comment 5

[Steven Johnson]

Attachment: Patch v3

Working patch, with selftest this time

Comment 6

[Stan Switzer]

(In reply to comment #0)
> There are lots of ad-hoc bits of code in Flash/AIR that prunes "dead" entries
> from lists of WeakRefs; we should add such a method to WeakRefList directly.

Agree, this is a nice addition.

Comment 7

[Lars T Hansen]

Comment on attachment 489307 [details] [diff] [review]
Patch v3

I think "(T)NULL" would be better than "(T)0".

If it were me I would not perform the ::store() unless kept < i, as that reduces write barrier traffic and generally makes it less expensive to call removeNullItems on lists that rarely see elements drop out.  Depends on how you expect it to be used, I guess, and how expensive you expect a barrier to be.

Comment 8

[Steven Johnson]

(In reply to comment #7)
> I think "(T)NULL" would be better than "(T)0".

Yeah, but newer gcc versions tend to be warning-prone about mismatching 0 and NULL; I thought 0 slightly less likely to generate warnings.
 
> If it were me I would not perform the ::store() unless kept < i,

Ah -- good catch. I'll test with that in mind and push that way if everything passes.

Comment 9

[Steven Johnson]

FWIW, using (T)NULL causes Xcode to generate this helpful warning:

    warning: NULL used in arithmetic

sigh...

Comment 10

[Steven Johnson]

pushed with kept<i, TR 5557:1497bb59b69d

Comment 11

[Tamarin Bot]

changeset: 5558:4f999233eeac
user:      Steven Johnson <[e-mail]>
date:      Wed Nov 17 13:52:02 2010 -0800
summary:   Bug 610022 (followup): I applied Lars optimization incorrectly (r=stejohns)

http://hg.mozilla.org/tamarin-redux/rev/4f999233eeac

Comment 12

[Tamarin Bot]

changeset: 5564:8abb0f8342eb
user:      Steven Johnson <[e-mail]>
summary:   Bug 610022 (followup 2): I *still* applied Lars optimization incorrectly (r=stejohns)

http://hg.mozilla.org/tamarin-redux/rev/8abb0f8342eb