When shrinking semispace don't relink pages if semispace is not committed

src/spaces.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1245 matching lines @@
   ASSERT((new_capacity & Page::kPageAlignmentMask) == 0);
   ASSERT(new_capacity >= initial_capacity_);
   ASSERT(new_capacity < capacity_);
   // Semispaces grow backwards from the end of their allocated capacity,
   // so we find the before and after start addresses relative to the
   // end of the space.
   Address space_end = start_ + maximum_capacity_;
   Address old_start = space_end - capacity_;
   size_t delta = capacity_ - new_capacity;
   ASSERT(IsAligned(delta, OS::AllocateAlignment()));
-  if (!heap()->isolate()->memory_allocator()->UncommitBlock(old_start, delta)) {
+  if (is_committed() &&
+      !heap()->isolate()->memory_allocator()->UncommitBlock(old_start, delta)) {
     return false;
   }
   capacity_ = new_capacity;
 
+  if (!is_committed()) {

[Michael Starzinger, 2012/01/04 14:38:28]

Can we invert the condition and place the block below inside the if statement? I
like that better than early returns.

[Sven Panne, 2012/01/04 14:46:54]

Drive-by comment: I *do* like guard clauses, they can make the normal control
flow much clearer.
(http://martinfowler.com/refactoring/catalog/replaceNestedConditionalWithGuard...)

Just my 2c... :-)

+    return true;
+  }
+
   int pages_after = capacity_ / Page::kPageSize;
   NewSpacePage* new_last_page =
       NewSpacePage::FromAddress(space_end - pages_after * Page::kPageSize);
   new_last_page->set_next_page(anchor());
   anchor()->set_prev_page(new_last_page);
   ASSERT((current_page_ <= first_page()) && (current_page_ >= new_last_page));
 
   return true;
 }
 
@@ skipping 1372 matching lines @@
     object->ShortPrint();
     PrintF("\n");
   }
   printf(" --------------------------------------\n");
   printf(" Marked: %x, LiveCount: %x\n", mark_size, LiveBytes());
 }
 
 #endif  // DEBUG
 
 } }  // namespace v8::internal