1186787 - Replace nsBaseHashtable::EnumerateRead() calls in chrome/ with iterators

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Nicholas Nethercote [inactive]]

Because iterators are so much nicer than enumerate functions.

There are two occurrences of EnumerateRead() in this directory.

A note to the assignee: to preserve existing behaviour, you should probably use
nsBaseHashtable::Iterator::UserData() rather than nsBaseHashtable::Iterator::Data(). (The latter should be used when replacing nsBaseHashtable::Enumerate()).

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Replace nsBaseHashtable::EnumerateRead() calls in chrome/ with iterators

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Replace nsBaseHashtable::EnumerateRead() calls in chrome/ with iterators

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8688253 [details] [diff] [review]
(part 2) - Replace nsBaseHashtable::EnumerateRead() calls in chrome/ with iterators

Review of attachment 8688253 [details] [diff] [review]:
-----------------------------------------------------------------

I wonder how much better things would be if we did:

  OverrideMapping* om = $.AppendElement();
  SerializeURI(..., om->originalURI);
  SerializeURI(..., om->overrideURI);

Not asking you to do this, just wondering about the micro-optimization.

Alternatively, for micro-optimizations, you could:

  OverrideMapping* omptr = $.AppendElements(mOverrideTable.Count());
  for (...) {
    SerializeURI(..., omptr->...);
    omptr++;
  }

Maybe all these micro-optimizations should be a separate bug. :)

Comment 4

[Nicholas Nethercote [inactive]]

> Maybe all these micro-optimizations should be a separate bug. :)

Feel free to file one :)  Measurements showing this code is hot would be a useful accompaniment; I suspect it's not but I've been wrong before...

Comment 5

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/401949c4008ed49464a7fd13781bee79b432a0b4
Bug 1186787 (part 1) - Replace nsBaseHashtable::EnumerateRead() calls in chrome/ with iterators. r=froydnj.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e0a66f7b693d055ef52c5b17d55cb117d6296b0
Bug 1186787 (part 2) - Replace nsBaseHashtable::EnumerateRead() calls in chrome/ with iterators. r=froydnj.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/401949c4008e
https://hg.mozilla.org/mozilla-central/rev/4e0a66f7b693