519156 - TM: fix the Tracker

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch

The Tracker currently has a performance bug.  It behaves differently on
64-bit machines -- a mask value is smaller -- for no good reason.  There's
a long comment explaining why the 64-bit case is different, but it's wrong.
I think that perhaps at one point in the past Tracker::map was the same size
on 32-bit and 64-bit platforms, and so it could only hold half as many
pointers in the 64-bit case, but that's no longer the case.

The result is that the top half of each page map allocated for the Tracker
is never touched, ie. some memory is needlessly allocated.  And also the
Tracker is more complicated than it needs to be.

This patch fixes the problem by using the same mask for 32-bits and 64-bits.
This is the right thing to do because in either case Tracker is dealing with
4-byte chunks of memory;  the only difference between the two cases is that
the Tracker::map is twice as big on 64-bits.  It also:

- Introduces TRACKER_PAGE_ENTRIES, which helps clarify things.

- Introduces getTrackerPageOffset(), to match getTrackerPageBase(), which
  further clarifies.

- Allocates the Tracker::map memory more straightforwardly, avoiding the
  unnecessary size arithmetic.

- Removes Tracker::clear() by inlining it into Tracker::~Tracker, the only
  function that uses clear().

- Uses NULL instead of 0 in a few places, which seems to be house style.

- Improves the comments -- removing the incorrect stuff, and also giving a
  brief high-level explanation of the Tracker.

Comment 1

[Andreas Gal :gal]

Comment on attachment 403175 [details] [diff] [review]
patch

Can you please leave clear() in there. I just started using that. Otherwise looks good.

Comment 2

[Nicholas Nethercote [inactive]]

Pushed with clear() reinstated:

http://hg.mozilla.org/tracemonkey/rev/c4c5e3586975

Comment 3

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/c4c5e3586975