Heap::gc_count_, last_gc_count, and kGCsBetweenCleanup should be unsigned...

src/heap.cc

Index: src/heap.cc
===================================================================
--- src/heap.cc	(revision 6057)
+++ src/heap.cc	(working copy)
@@ -133,7 +133,7 @@
 
 int Heap::mc_count_ = 0;
 int Heap::ms_count_ = 0;
-int Heap::gc_count_ = 0;
+unsigned int Heap::gc_count_ = 0;
 
 GCTracer* Heap::tracer_ = NULL;
 
@@ -3758,9 +3758,9 @@
   static const int kIdlesBeforeMarkSweep = 7;
   static const int kIdlesBeforeMarkCompact = 8;
   static const int kMaxIdleCount = kIdlesBeforeMarkCompact + 1;
-  static const int kGCsBetweenCleanup = 4;
+  static const unsigned int kGCsBetweenCleanup = 4;
   static int number_idle_notifications = 0;
-  static int last_gc_count = gc_count_;
+  static unsigned int last_gc_count = gc_count_;

[Vyacheslav Egorov (Chromium), 2011/02/21 15:35:14]

There is a bug here: last_gc_count will be initialized once (on the first
invocation of IdleNotification) with gc_count_ value and will not be
reinitialized again so it will always remain the same.

Please split this into declaration and assignment.

And thanks for making me notice this place.

 
   bool uncommit = true;
   bool finished = false;
@@ -3769,7 +3769,7 @@
   // GCs have taken place. This allows another round of cleanup based
   // on idle notifications if enough work has been carried out to
   // provoke a number of garbage collections.
-  if (gc_count_ < last_gc_count + kGCsBetweenCleanup) {
+  if (gc_count_ - last_gc_count < kGCsBetweenCleanup) {
     number_idle_notifications =
         Min(number_idle_notifications + 1, kMaxIdleCount);
   } else {