Simplify partitionReallocGeneric() implementation

Source/wtf/PartitionAlloc.cpp

 /*
  * Copyright (C) 2013 Google Inc. 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
@@ skipping 757 matching lines @@
 #if defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
     return realloc(ptr, newSize);
 #else
     if (UNLIKELY(!ptr))
         return partitionAllocGeneric(root, newSize);
     if (UNLIKELY(!newSize)) {
         partitionFreeGeneric(root, ptr);
         return 0;
     }
 
-    // TODO: note that tcmalloc will "ignore" a downsizing realloc() unless the

[Chris Evans, 2014/02/26 00:22:52]

This TODO is not TO-DONE :) so I'm not sure why it is removed?

[Jens Widell, 2014/02/26 05:45:44]

Sorry about that; quite sloppy of me. Will restore.

-    // new size is a significant percentage smaller. We could do the same if we
-    // determine it is a win.
-    void* realPtr = partitionCookieFreePointerAdjust(ptr);
-    ASSERT(partitionPointerIsValid(realPtr));
-    PartitionPage* oldPage = partitionPointerToPage(realPtr);
-    PartitionBucket* oldBucket = oldPage->bucket;
+    ASSERT(partitionPointerIsValid(partitionCookieFreePointerAdjust(ptr)));
 
-    size_t allocSize = partitionCookieSizeAdjustAdd(newSize);
-    PartitionBucket* newBucket = partitionGenericSizeToBucket(root, allocSize);
+    size_t actualNewSize = partitionAllocActualSize(root, newSize);

[Chris Evans, 2014/02/26 00:22:52]

Don't we need partitionCookieSizeAdjustAdd() around newSize?

[Jens Widell, 2014/02/26 05:45:44]

We don't need it for the comparison to actualOldSize; both values are actual
usable space and thus "inside" the cookies.

There are partitionCookieSizeAdjust{Add,Subtract} calls first and last in
partitionAllocActualSize().

+    size_t actualOldSize = partitionAllocGetSize(ptr);
 
-    // TODO: for a downsize on a direct mapped allocation, we really should

[Chris Evans, 2014/02/26 00:22:52]

This TODO is also not TO-DONE. So we might want to keep it. But can you fix it?
It should say "for an upsize or downsize on a direct mapped allocation, we
should really try and resize it in-place".

-    // just de-commit the correct number of pages off the end.
-    if (oldBucket == newBucket)
+    if (actualNewSize == actualOldSize) {
+        // Trying to allocate a block of size newSize would give us a block of
+        // the same size as the one we've already got, so no point in doing
+        // anything here.
         return ptr;
+    }
 
     // This realloc cannot be resized in-place. Sadness.
     void* ret = partitionAllocGeneric(root, newSize);
-    size_t copySize = oldPage->bucket->slotSize;
-    copySize = partitionCookieSizeAdjustSubtract(copySize);
+    size_t copySize = actualOldSize;

[Chris Evans, 2014/02/26 00:22:52]

I think the loss of the partitionCookieSizeAdjustSubtract() call here might
cause failure in debug mode?

[Jens Widell, 2014/02/26 05:45:44]

I don't see it. This function is now only using "public" functions, so ought to
be unaffected by the cookie details the same way other client code using the
public API is.

Looking at the tests I can't really see any checks that we actually copied all
the bytes we were supposed to, only checks that we copied the first byte, and
one check that we didn't copy too much. I should increase coverage a bit along
with this patch.

     if (newSize < copySize)
         copySize = newSize;
 
     memcpy(ret, ptr, copySize);
     partitionFreeGeneric(root, ptr);
     return ret;
 #endif
 }
 
 #ifndef NDEBUG
@@ skipping 47 matching lines @@
     printf("total live: %zu bytes\n", totalLive);
     printf("total resident: %zu bytes\n", totalResident);
     printf("total freeable: %zu bytes\n", totalFreeable);
     fflush(stdout);
 }
 
 #endif // !NDEBUG
 
 } // namespace WTF