1506730 - remove PLDHashTable::Iterator::mStart

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nathan Froyd [:froydnj]]

We only use its value in one place, and said value is easily computable
from readily available information.  This change makes iterators
slightly smaller.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: remove PLDHashTable::Iterator::mStart

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 9024598 [details] [diff] [review]
remove PLDHashTable::Iterator::mStart

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

Iterators are always allocated on the stack, so making them smaller isn't much of a win.

Have you measured the benchmarks in xpcom/rust/gtest/bench-collections/Bench.cpp? When I was working on them I found that the iteration benchmark was highly sensitive to very minor changes in the code.

Comment 3

[Nathan Froyd [:froydnj]]

I don't really understand the results, but apparently removing mStart makes a small (positive) difference.

Without patch:

[ RUN      ] BenchCollections.PLDHash

  succ_lookups  fail_lookups insert_remove       iterate         total
       96.8 ms       75.6 ms       26.2 ms       42.6 ms      241.3 ms
       52.3 ms       55.9 ms       22.0 ms       39.8 ms      170.0 ms
       52.4 ms       56.0 ms       21.8 ms       39.7 ms      170.0 ms
       52.3 ms       56.2 ms       22.1 ms       40.6 ms      171.2 ms
       51.8 ms       56.3 ms       22.0 ms       39.8 ms      170.0 ms

With patch:

[ RUN      ] BenchCollections.PLDHash

  succ_lookups  fail_lookups insert_remove       iterate         total
       73.2 ms       65.2 ms       23.7 ms       40.4 ms      202.5 ms
       51.1 ms       55.5 ms       21.8 ms       40.2 ms      168.6 ms
       51.2 ms       56.2 ms       22.0 ms       40.1 ms      169.4 ms
       51.2 ms       55.3 ms       21.9 ms       38.3 ms      166.7 ms
       48.6 ms       52.2 ms       20.0 ms       38.1 ms      158.9 ms

These results make zero sense to me; it's possible I'm not running them on a quiet enough machine or something?  OTOH, I can run the benchmark repeatedly with the same state and get differences of 10%, so I think the results fall in the noise.

Comment 4

[Nicholas Nethercote [inactive]]

Comment on attachment 9024598 [details] [diff] [review]
remove PLDHashTable::Iterator::mStart

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

The results are noisy in general, but the "iterate" result didn't change, so I'm happy with this.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/640b09ec036e
remove PLDHashTable::Iterator::mStart; r=njn

Comment 6

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/640b09ec036e