From 918090c819109585f8fd2295039385a60eb0b572 Mon Sep 17 00:00:00 2001 From: bungeman Date: Tue, 9 Feb 2016 09:14:28 -0800 Subject: [PATCH] SkTArray to move when moving. This updates SkTArray to move elements when possible, instead of always copying them. TBR=reed Agreed moving is good. This should also become private. Committed: https://skia.googlesource.com/skia/+/3c69348e725131150e4ab962dec1b3ff1148a6bd Review URL: https://codereview.chromium.org/1672063002 --- gyp/common_conditions.gypi | 2 + include/core/SkTArray.h | 84 +++++++++++++-------------------- include/ports/SkFontMgr_indirect.h | 3 +- src/ports/SkRemotableFontMgr_win_dw.cpp | 5 +- 4 files changed, 38 insertions(+), 56 deletions(-) diff --git a/gyp/common_conditions.gypi b/gyp/common_conditions.gypi index 163f1b4..85f3e20 100644 --- a/gyp/common_conditions.gypi +++ b/gyp/common_conditions.gypi @@ -178,6 +178,8 @@ 'WarnAsError': 'true', 'AdditionalOptions': [ '/we4189', # initialized but unused var warning + '/we4238', # taking address of rvalue + '/we4239', # assigning rvalues to non-const lvalues ], }, }, diff --git a/include/core/SkTArray.h b/include/core/SkTArray.h index bd255e3..86c5eaa 100644 --- a/include/core/SkTArray.h +++ b/include/core/SkTArray.h @@ -8,6 +8,7 @@ #ifndef SkTArray_DEFINED #define SkTArray_DEFINED +#include "../private/SkTLogic.h" #include "../private/SkTemplates.h" #include "SkTypes.h" @@ -15,42 +16,6 @@ #include template class SkTArray; - -namespace SkTArrayExt { - -template -inline void copy(SkTArray* self, int dst, int src) { - memcpy(&self->fItemArray[dst], &self->fItemArray[src], sizeof(T)); -} -template -inline void copy(SkTArray* self, const T* array) { - sk_careful_memcpy(self->fMemArray, array, self->fCount * sizeof(T)); -} -template -inline void copyAndDelete(SkTArray* self, char* newMemArray) { - sk_careful_memcpy(newMemArray, self->fMemArray, self->fCount * sizeof(T)); -} - -template -inline void copy(SkTArray* self, int dst, int src) { - new (&self->fItemArray[dst]) T(self->fItemArray[src]); -} -template -inline void copy(SkTArray* self, const T* array) { - for (int i = 0; i < self->fCount; ++i) { - new (self->fItemArray + i) T(array[i]); - } -} -template -inline void copyAndDelete(SkTArray* self, char* newMemArray) { - for (int i = 0; i < self->fCount; ++i) { - new (newMemArray + sizeof(T) * i) T(self->fItemArray[i]); - self->fItemArray[i].~T(); - } -} - -} - template void* operator new(size_t, SkTArray*, int); /** When MEM_COPY is true T will be bit copied when moved. @@ -105,7 +70,7 @@ public: fCount = 0; this->checkRealloc((int)array.count()); fCount = array.count(); - SkTArrayExt::copy(this, static_cast(array.fMemArray)); + this->copy(static_cast(array.fMemArray)); return *this; } @@ -150,7 +115,7 @@ public: int delta = count - fCount; this->checkRealloc(delta); fCount = count; - SkTArrayExt::copy(this, array); + this->copy(array); } void removeShuffle(int n) { @@ -159,8 +124,7 @@ public: fCount = newCount; fItemArray[n].~T(); if (n != newCount) { - SkTArrayExt::copy(this, n, newCount); - fItemArray[newCount].~T(); + this->move(n, newCount); } } @@ -422,10 +386,38 @@ protected: fMemArray = sk_malloc_throw(fAllocCount * sizeof(T)); } - SkTArrayExt::copy(this, array); + this->copy(array); } private: + /** In the following move and copy methods, 'dst' is assumed to be uninitialized raw storage. + * In the following move methods, 'src' is destroyed leaving behind uninitialized raw storage. + */ + template SK_WHEN(E, void) copy(const T* src) { + sk_careful_memcpy(fMemArray, src, fCount * sizeof(T)); + } + template SK_WHEN(E, void) move(int dst, int src) { + memcpy(&fItemArray[dst], &fItemArray[src], sizeof(T)); + } + template SK_WHEN(E, void) move(char* dst) { + sk_careful_memcpy(dst, fMemArray, fCount * sizeof(T)); + } + + template SK_WHEN(!E, void) copy(const T* src) { + for (int i = 0; i < fCount; ++i) { + new (fItemArray + i) T(src[i]); + } + } + template SK_WHEN(!E, void) move(int dst, int src) { + new (&fItemArray[dst]) T(std::move(fItemArray[src])); + fItemArray[src].~T(); + } + template SK_WHEN(!E, void) move(char* dst) { + for (int i = 0; i < fCount; ++i) { + new (dst + sizeof(T) * i) T(std::move(fItemArray[i])); + fItemArray[i].~T(); + } + } static const int gMIN_ALLOC_COUNT = 8; @@ -463,7 +455,7 @@ private: newMemArray = (char*) sk_malloc_throw(fAllocCount*sizeof(T)); } - SkTArrayExt::copyAndDelete(this, newMemArray); + this->move(newMemArray); if (fMemArray != fPreAllocMemArray) { sk_free(fMemArray); @@ -474,14 +466,6 @@ private: friend void* operator new(size_t, SkTArray*, int); - template friend void SkTArrayExt::copy(SkTArray* that, int dst, int src); - template friend void SkTArrayExt::copy(SkTArray* that, const X*); - template friend void SkTArrayExt::copyAndDelete(SkTArray* that, char*); - - template friend void SkTArrayExt::copy(SkTArray* that, int dst, int src); - template friend void SkTArrayExt::copy(SkTArray* that, const X*); - template friend void SkTArrayExt::copyAndDelete(SkTArray* that, char*); - int fReserveCount; int fCount; int fAllocCount; diff --git a/include/ports/SkFontMgr_indirect.h b/include/ports/SkFontMgr_indirect.h index 81cd773..9d96fee 100644 --- a/include/ports/SkFontMgr_indirect.h +++ b/include/ports/SkFontMgr_indirect.h @@ -70,8 +70,7 @@ private: DataEntry() { } - // This is a move!!! - DataEntry(DataEntry& that) + DataEntry(DataEntry&& that) : fDataId(that.fDataId) , fTtcIndex(that.fTtcIndex) , fTypeface(that.fTypeface) diff --git a/src/ports/SkRemotableFontMgr_win_dw.cpp b/src/ports/SkRemotableFontMgr_win_dw.cpp index 6c021a1..d200173 100644 --- a/src/ports/SkRemotableFontMgr_win_dw.cpp +++ b/src/ports/SkRemotableFontMgr_win_dw.cpp @@ -32,10 +32,7 @@ private: DataId() { } - // This is actually a move!!! - explicit DataId(DataId& that) - : fLoader(that.fLoader), fKey(that.fKey), fKeySize(that.fKeySize) - { + DataId(DataId&& that) : fLoader(that.fLoader), fKey(that.fKey), fKeySize(that.fKeySize) { that.fLoader = nullptr; that.fKey = nullptr; SkDEBUGCODE(that.fKeySize = 0xFFFFFFFF;) -- 2.7.4