diff options
author | erg@google.com <erg@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-19 17:04:46 +0000 |
---|---|---|
committer | erg@google.com <erg@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-19 17:04:46 +0000 |
commit | 6de2799a47cbc201d943b32e0ac62555dc45f662 (patch) | |
tree | 6c64342df90783c5f334d370d6f8cb08472fcd7f /app | |
parent | 5f1b13bc6952eb89149716930e654c9d989b9e13 (diff) | |
download | chromium_src-6de2799a47cbc201d943b32e0ac62555dc45f662.zip chromium_src-6de2799a47cbc201d943b32e0ac62555dc45f662.tar.gz chromium_src-6de2799a47cbc201d943b32e0ac62555dc45f662.tar.bz2 |
First fix to minimize copying of image data.
This is the first of multiple patches that clean up handling of memory
regarding images. Previously, the code did several memcpy()s or
equivalents. This change:
- Creates an abstract interface RefCountedMemory which provides access to the
front() of a memory range and the size() of it. It is a RefCountedThreadSafe.
- Adds a RefCountedStaticMemory class which isa RefCountedMemory.
- Pushes RefCountedBytes up into base/ from chrome/ and make it conform to
RefCountedMemory.
- Have ResourceBundle return RefCountedStaticMemory to the mmaped() or DLL
loaded resources instead of memcpy()ing them.
- General cleanups to minimize copies in constructing RefCountedBytes.
- Use the above consistent interface in the BrowserThemeProvider, along with
special casing the loading of the new tab page background.
This patch is mostly cleanups and there should only be a slight performance
gain if any. Most of the real speedups should come in subsequent patches.
BUG=http://crbug.com/24493
TEST=Slightly faster on Perf bot; does not introduce crashes.
Review URL: http://codereview.chromium.org/288005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29412 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'app')
-rw-r--r-- | app/resource_bundle.cc | 28 | ||||
-rw-r--r-- | app/resource_bundle.h | 19 | ||||
-rw-r--r-- | app/resource_bundle_linux.cc | 28 | ||||
-rw-r--r-- | app/resource_bundle_mac.mm | 18 | ||||
-rw-r--r-- | app/resource_bundle_win.cc | 15 | ||||
-rw-r--r-- | app/theme_provider.h | 8 |
6 files changed, 54 insertions, 62 deletions
diff --git a/app/resource_bundle.cc b/app/resource_bundle.cc index a108660..7261dfd 100644 --- a/app/resource_bundle.cc +++ b/app/resource_bundle.cc @@ -69,20 +69,20 @@ void ResourceBundle::FreeImages() { /* static */ SkBitmap* ResourceBundle::LoadBitmap(DataHandle data_handle, int resource_id) { - std::vector<unsigned char> raw_data, png_data; - bool success = false; + std::vector<unsigned char> png_data; - if (!success) - success = LoadResourceBytes(data_handle, resource_id, &raw_data); - if (!success) + scoped_refptr<RefCountedMemory> memory( + LoadResourceBytes(data_handle, resource_id)); + if (!memory) return NULL; // Decode the PNG. int image_width; int image_height; - if (!gfx::PNGCodec::Decode(&raw_data.front(), raw_data.size(), - gfx::PNGCodec::FORMAT_BGRA, - &png_data, &image_width, &image_height)) { + if (!gfx::PNGCodec::Decode( + memory->front(), memory->size(), + gfx::PNGCodec::FORMAT_BGRA, + &png_data, &image_width, &image_height)) { NOTREACHED() << "Unable to decode image resource " << resource_id; return NULL; } @@ -96,14 +96,14 @@ std::string ResourceBundle::GetDataResource(int resource_id) { return GetRawDataResource(resource_id).as_string(); } -bool ResourceBundle::LoadImageResourceBytes(int resource_id, - std::vector<unsigned char>* bytes) { - return LoadResourceBytes(theme_data_, resource_id, bytes); +RefCountedStaticMemory* ResourceBundle::LoadImageResourceBytes( + int resource_id) { + return LoadResourceBytes(theme_data_, resource_id); } -bool ResourceBundle::LoadDataResourceBytes(int resource_id, - std::vector<unsigned char>* bytes) { - return LoadResourceBytes(resources_data_, resource_id, bytes); +RefCountedStaticMemory* ResourceBundle::LoadDataResourceBytes( + int resource_id) { + return LoadResourceBytes(resources_data_, resource_id); } SkBitmap* ResourceBundle::GetBitmapNamed(int resource_id) { diff --git a/app/resource_bundle.h b/app/resource_bundle.h index 23f39c5..f2fdb58 100644 --- a/app/resource_bundle.h +++ b/app/resource_bundle.h @@ -17,6 +17,7 @@ #include "base/basictypes.h" #include "base/file_path.h" #include "base/lock.h" +#include "base/ref_counted_memory.h" #include "base/scoped_ptr.h" #include "base/string16.h" @@ -90,14 +91,12 @@ class ResourceBundle { // Loads the raw bytes of an image resource into |bytes|, // without doing any processing or interpretation of // the resource. Returns whether we successfully read the resource. - bool LoadImageResourceBytes(int resource_id, - std::vector<unsigned char>* bytes); + RefCountedStaticMemory* LoadImageResourceBytes(int resource_id); // Loads the raw bytes of a data resource into |bytes|, // without doing any processing or interpretation of // the resource. Returns whether we successfully read the resource. - bool LoadDataResourceBytes(int resource_id, - std::vector<unsigned char>* bytes); + RefCountedStaticMemory* LoadDataResourceBytes(int resource_id); // Return the contents of a file in a string given the resource id. // This will copy the data from the resource and return it as a string. @@ -185,12 +184,12 @@ class ResourceBundle { // string if no locale data files are found. FilePath GetLocaleFilePath(const std::wstring& pref_locale); - // Loads the raw bytes of a resource from |module| into |bytes|, - // without doing any processing or interpretation of - // the resource. Returns whether we successfully read the resource. - static bool LoadResourceBytes(DataHandle module, - int resource_id, - std::vector<unsigned char>* bytes); + // Returns a handle to bytes from the resource |module|, without doing any + // processing or interpretation of the resource. Returns whether we + // successfully read the resource. Caller does not own the data returned + // through this method and must not modify the data pointed to by |bytes|. + static RefCountedStaticMemory* LoadResourceBytes(DataHandle module, + int resource_id); // Creates and returns a new SkBitmap given the data file to look in and the // resource id. It's up to the caller to free the returned bitmap when diff --git a/app/resource_bundle_linux.cc b/app/resource_bundle_linux.cc index 911cc03..929a77f 100644 --- a/app/resource_bundle_linux.cc +++ b/app/resource_bundle_linux.cc @@ -25,10 +25,10 @@ namespace { // Convert the raw image data into a GdkPixbuf. The GdkPixbuf that is returned // has a ref count of 1 so the caller must call g_object_unref to free the // memory. -GdkPixbuf* LoadPixbuf(std::vector<unsigned char>& data, bool rtl_enabled) { +GdkPixbuf* LoadPixbuf(RefCountedStaticMemory* data, bool rtl_enabled) { ScopedGObject<GdkPixbufLoader>::Type loader(gdk_pixbuf_loader_new()); bool ok = gdk_pixbuf_loader_write(loader.get(), - static_cast<guint8*>(data.data()), data.size(), NULL); + reinterpret_cast<const guint8*>(data->front()), data->size(), NULL); if (!ok) return NULL; // Calling gdk_pixbuf_loader_close forces the data to be parsed by the @@ -117,18 +117,16 @@ void ResourceBundle::LoadThemeResources() { DCHECK(success) << "failed to load theme data"; } -/* static */ -bool ResourceBundle::LoadResourceBytes(DataHandle module, int resource_id, - std::vector<unsigned char>* bytes) { +// static +RefCountedStaticMemory* ResourceBundle::LoadResourceBytes( + DataHandle module, int resource_id) { DCHECK(module); - base::StringPiece data; - if (!module->Get(resource_id, &data)) - return false; - - bytes->resize(data.length()); - memcpy(&(bytes->front()), data.data(), data.length()); + base::StringPiece bytes; + if (!module->Get(resource_id, &bytes)) + return NULL; - return true; + return new RefCountedStaticMemory( + reinterpret_cast<const unsigned char*>(bytes.data()), bytes.length()); } base::StringPiece ResourceBundle::GetRawDataResource(int resource_id) { @@ -176,9 +174,9 @@ GdkPixbuf* ResourceBundle::GetPixbufImpl(int resource_id, bool rtl_enabled) { return found->second; } - std::vector<unsigned char> data; - LoadImageResourceBytes(resource_id, &data); - GdkPixbuf* pixbuf = LoadPixbuf(data, rtl_enabled); + scoped_refptr<RefCountedStaticMemory> data( + LoadImageResourceBytes(resource_id)); + GdkPixbuf* pixbuf = LoadPixbuf(data.get(), rtl_enabled); // We loaded successfully. Cache the pixbuf. if (pixbuf) { diff --git a/app/resource_bundle_mac.mm b/app/resource_bundle_mac.mm index 6696617..b306602 100644 --- a/app/resource_bundle_mac.mm +++ b/app/resource_bundle_mac.mm @@ -71,18 +71,16 @@ void ResourceBundle::LoadThemeResources() { DCHECK(theme_data_) << "failed to load theme.pak"; } -/* static */ -bool ResourceBundle::LoadResourceBytes(DataHandle module, int resource_id, - std::vector<unsigned char>* bytes) { +// static +RefCountedStaticMemory* ResourceBundle::LoadResourceBytes( + DataHandle module, int resource_id) { DCHECK(module); - base::StringPiece data; - if (!module->Get(resource_id, &data)) - return false; - - bytes->resize(data.length()); - memcpy(&(bytes->front()), data.data(), data.length()); + base::StringPiece bytes; + if (!module->Get(resource_id, &bytes)) + return NULL; - return true; + return new RefCountedStaticMemory( + reinterpret_cast<const unsigned char*>(bytes.data()), bytes.length()); } base::StringPiece ResourceBundle::GetRawDataResource(int resource_id) { diff --git a/app/resource_bundle_win.cc b/app/resource_bundle_win.cc index fc9e360..0c25c47 100644 --- a/app/resource_bundle_win.cc +++ b/app/resource_bundle_win.cc @@ -85,20 +85,17 @@ void ResourceBundle::LoadThemeResources() { DCHECK(theme_data_ != NULL) << "unable to load " << theme_data_path.value(); } -/* static */ -bool ResourceBundle::LoadResourceBytes( - DataHandle module, - int resource_id, - std::vector<unsigned char>* bytes) { +// static +RefCountedStaticMemory* ResourceBundle::LoadResourceBytes( + DataHandle module, int resource_id) { void* data_ptr; size_t data_size; if (base::GetDataResourceFromModule(module, resource_id, &data_ptr, &data_size)) { - bytes->resize(data_size); - memcpy(&(bytes->front()), data_ptr, data_size); - return true; + return new RefCountedStaticMemory( + reinterpret_cast<const unsigned char*>(data_ptr), data_size); } else { - return false; + return NULL; } } diff --git a/app/theme_provider.h b/app/theme_provider.h index c3b8a9c..410e699 100644 --- a/app/theme_provider.h +++ b/app/theme_provider.h @@ -23,6 +23,7 @@ class NSImage; #endif // OS_* class Profile; +class RefCountedMemory; class SkBitmap; //////////////////////////////////////////////////////////////////////////////// @@ -60,10 +61,9 @@ class ThemeProvider { // doesn't provide a certain image, but custom themes might (badges, etc). virtual bool HasCustomImage(int id) const = 0; - // Reads the image data from the theme file into the specified vector. Returns - // true on success. - virtual bool GetRawData(int id, - std::vector<unsigned char>* raw_data) const = 0; + // Reads the image data from the theme file into the specified + // vector. Returns NULL on error. + virtual RefCountedMemory* GetRawData(int id) const = 0; #if defined(OS_LINUX) && !defined(TOOLKIT_VIEWS) // Gets the GdkPixbuf with the specified |id|. Returns a pointer to a shared |