From 6bb76947aef081b9143236c3de34a273d9126ffe Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 12 Apr 2023 13:38:29 -0700 Subject: [PATCH] Do not pass std::shared_ptr when not transfering ownership This goes against Google's C++ primer [1] and the Core C++ guidelines [2]. It incurs additional runtime overhead to increase and subsequently decrease the reference count without providing value, since the parent function maintains the a reference to the object through the duration of the function. 1: go/cpp-primer#unique_ptr - "In general, if you find yourself wanting to use a pointer or reference to a unique_ptr, you're probably not transferring ownership, so you should usually just pass a raw pointer or reference to the underlying object, and keep unique_ptr out of it." 2: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f7-for-general-use-take-t-or-t-arguments-rather-than-smart-pointers Reviewed-by: Aaron Ruby Acked-by: Yonggang Luo Acked-by: Adam Jackson Part-of: --- .../VirtioGpuAddressSpaceStream.cpp | 2 +- src/gfxstream/guest/platform/fuchsia/FuchsiaVirtGpu.h | 8 ++++---- .../guest/platform/fuchsia/FuchsiaVirtGpuBlob.cpp | 4 ++-- .../guest/platform/fuchsia/FuchsiaVirtGpuDevice.cpp | 3 ++- src/gfxstream/guest/platform/include/VirtGpu.h | 10 +++++----- src/gfxstream/guest/platform/linux/LinuxVirtGpu.h | 8 ++++---- .../guest/platform/linux/LinuxVirtGpuBlob.cpp | 8 ++++---- .../guest/platform/linux/LinuxVirtGpuDevice.cpp | 2 +- src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp | 2 +- 9 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/gfxstream/guest/GoldfishAddressSpace/VirtioGpuAddressSpaceStream.cpp b/src/gfxstream/guest/GoldfishAddressSpace/VirtioGpuAddressSpaceStream.cpp index 51e79724b0b..7f6023e5a3b 100644 --- a/src/gfxstream/guest/GoldfishAddressSpace/VirtioGpuAddressSpaceStream.cpp +++ b/src/gfxstream/guest/GoldfishAddressSpace/VirtioGpuAddressSpaceStream.cpp @@ -112,7 +112,7 @@ AddressSpaceStream* createVirtioGpuAddressSpaceStream(enum VirtGpuCapset capset, exec.command = static_cast(&contextCreate); exec.command_size = sizeof(contextCreate); - ret = instance->execBuffer(exec, blob); + ret = instance->execBuffer(exec, blob.get()); if (ret) return nullptr; diff --git a/src/gfxstream/guest/platform/fuchsia/FuchsiaVirtGpu.h b/src/gfxstream/guest/platform/fuchsia/FuchsiaVirtGpu.h index bf3089c429b..10f6d40387f 100644 --- a/src/gfxstream/guest/platform/fuchsia/FuchsiaVirtGpu.h +++ b/src/gfxstream/guest/platform/fuchsia/FuchsiaVirtGpu.h @@ -25,9 +25,9 @@ class FuchsiaVirtGpuBlob : public std::enable_shared_from_this, uint64_t size); ~LinuxVirtGpuBlob(); - uint32_t getResourceHandle(void) override; - uint32_t getBlobHandle(void) override; - int wait(void) override; + uint32_t getResourceHandle() const override; + uint32_t getBlobHandle() const override; + int wait() override; VirtGpuBlobMappingPtr createMapping(void) override; int exportBlob(struct VirtGpuExternalHandle& handle) override; @@ -70,7 +70,7 @@ class LinuxVirtGpuDevice : public VirtGpuDevice { VirtGpuBlobPtr createVirglBlob(uint32_t width, uint32_t height, uint32_t virglFormat) override; virtual VirtGpuBlobPtr importBlob(const struct VirtGpuExternalHandle& handle); - virtual int execBuffer(struct VirtGpuExecBuffer& execbuffer, VirtGpuBlobPtr blob); + virtual int execBuffer(struct VirtGpuExecBuffer& execbuffer, const VirtGpuBlob* blob); private: int64_t mDeviceHandle; diff --git a/src/gfxstream/guest/platform/linux/LinuxVirtGpuBlob.cpp b/src/gfxstream/guest/platform/linux/LinuxVirtGpuBlob.cpp index 0d5035fbbbb..9ba1adcd52c 100644 --- a/src/gfxstream/guest/platform/linux/LinuxVirtGpuBlob.cpp +++ b/src/gfxstream/guest/platform/linux/LinuxVirtGpuBlob.cpp @@ -33,7 +33,7 @@ LinuxVirtGpuBlob::LinuxVirtGpuBlob(int64_t deviceHandle, uint32_t blobHandle, mResourceHandle(resourceHandle), mSize(size) {} -LinuxVirtGpuBlob::~LinuxVirtGpuBlob(void) { +LinuxVirtGpuBlob::~LinuxVirtGpuBlob() { struct drm_gem_close gem_close { .handle = mBlobHandle, .pad = 0, }; @@ -45,11 +45,11 @@ LinuxVirtGpuBlob::~LinuxVirtGpuBlob(void) { } } -uint32_t LinuxVirtGpuBlob::getBlobHandle(void) { return mBlobHandle; } +uint32_t LinuxVirtGpuBlob::getBlobHandle() const { return mBlobHandle; } -uint32_t LinuxVirtGpuBlob::getResourceHandle(void) { return mResourceHandle; } +uint32_t LinuxVirtGpuBlob::getResourceHandle() const { return mResourceHandle; } -VirtGpuBlobMappingPtr LinuxVirtGpuBlob::createMapping(void) { +VirtGpuBlobMappingPtr LinuxVirtGpuBlob::createMapping() { int ret; struct drm_virtgpu_map map { .handle = mBlobHandle, .pad = 0, diff --git a/src/gfxstream/guest/platform/linux/LinuxVirtGpuDevice.cpp b/src/gfxstream/guest/platform/linux/LinuxVirtGpuDevice.cpp index 9e35cdbc44f..a54abfcf51a 100644 --- a/src/gfxstream/guest/platform/linux/LinuxVirtGpuDevice.cpp +++ b/src/gfxstream/guest/platform/linux/LinuxVirtGpuDevice.cpp @@ -245,7 +245,7 @@ VirtGpuBlobPtr LinuxVirtGpuDevice::importBlob(const struct VirtGpuExternalHandle static_cast(info.size)); } -int LinuxVirtGpuDevice::execBuffer(struct VirtGpuExecBuffer& execbuffer, VirtGpuBlobPtr blob) { +int LinuxVirtGpuDevice::execBuffer(struct VirtGpuExecBuffer& execbuffer, const VirtGpuBlob* blob) { int ret; struct drm_virtgpu_execbuffer exec = {0}; uint32_t blobHandle; diff --git a/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp b/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp index b2affa9c2e2..ae997c88079 100644 --- a/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp +++ b/src/gfxstream/guest/vulkan_enc/ResourceTracker.cpp @@ -3068,7 +3068,7 @@ VkResult ResourceTracker::allocateCoherentMemory(VkDevice device, exec.command_size = sizeof(placeholderCmd); exec.flags = kRingIdx; exec.ring_idx = 1; - if (instance->execBuffer(exec, guestBlob)) { + if (instance->execBuffer(exec, guestBlob.get())) { ALOGE("Failed to allocate coherent memory: failed to execbuffer for wait."); return VK_ERROR_OUT_OF_HOST_MEMORY; }