From 766c74055a892b0b6759f57bc3b9bbbaf523386e Mon Sep 17 00:00:00 2001
From: chengduoZH <zhaochengduo@163.com>
Date: Tue, 3 Apr 2018 19:25:29 +0800
Subject: [PATCH] follow comments

---
 paddle/fluid/memory/CMakeLists.txt            |  6 +++---
 paddle/fluid/memory/detail/system_allocator.h |  8 ++++++--
 paddle/fluid/memory/pinned_memory_test.cu     | 18 +++++++++++-------
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/paddle/fluid/memory/CMakeLists.txt b/paddle/fluid/memory/CMakeLists.txt
index 53fd8626fa2..66e4f8cdd02 100644
--- a/paddle/fluid/memory/CMakeLists.txt
+++ b/paddle/fluid/memory/CMakeLists.txt
@@ -15,6 +15,6 @@ cc_library(paddle_memory
 
 cc_test(memory_test SRCS memory_test.cc DEPS place paddle_memory)
 
-if (WITH_GPU)
-    nv_test(pinned_memory_test SRCS pinned_memory_test.cu  DEPS place paddle_memory)
-endif()
+# if (WITH_GPU)
+#    nv_test(pinned_memory_test SRCS pinned_memory_test.cu  DEPS place paddle_memory)
+# endif()
diff --git a/paddle/fluid/memory/detail/system_allocator.h b/paddle/fluid/memory/detail/system_allocator.h
index c2f474f4b66..e3c50ef6483 100644
--- a/paddle/fluid/memory/detail/system_allocator.h
+++ b/paddle/fluid/memory/detail/system_allocator.h
@@ -21,8 +21,9 @@ namespace memory {
 namespace detail {
 
 /**
- * \brief SystemAllocator is the parent class of CPUAllocator and GPUAllocator.
- *        A BuddyAllocator object uses a SystemAllocator* pointing to the
+ * \brief SystemAllocator is the parent class of CPUAllocator,
+ *        CUDAPinnedAllocator and GPUAllocator. A BuddyAllocator
+ *        object uses a SystemAllocator* pointing to the
  *        underlying system allocator.
  */
 class SystemAllocator {
@@ -43,6 +44,8 @@ class CPUAllocator : public SystemAllocator {
 #ifdef PADDLE_WITH_CUDA
 class GPUAllocator : public SystemAllocator {
  public:
+  explicit GPUAllocator(int gpu_id) : gpu_id_(gpu_id) {}
+
   virtual void* Alloc(size_t& index, size_t size);
   virtual void Free(void* p, size_t size, size_t index);
   virtual bool UseGpu() const;
@@ -50,6 +53,7 @@ class GPUAllocator : public SystemAllocator {
  private:
   size_t gpu_alloc_size_ = 0;
   size_t fallback_alloc_size_ = 0;
+  int gpu_id_;
 };
 
 class CUDAPinnedAllocator : public SystemAllocator {
diff --git a/paddle/fluid/memory/pinned_memory_test.cu b/paddle/fluid/memory/pinned_memory_test.cu
index cba39cd4072..926a5b265bb 100644
--- a/paddle/fluid/memory/pinned_memory_test.cu
+++ b/paddle/fluid/memory/pinned_memory_test.cu
@@ -24,6 +24,8 @@ limitations under the License. */
 #include <gtest/gtest.h>
 #include <unordered_map>
 
+// This unit test is an example comparing the performance between using pinned
+// memory and not. In general, using pinned memory will be faster.
 template <typename T>
 __global__ void Kernel(T* output, int dim) {
   int tid = blockIdx.x * blockDim.x + threadIdx.x;
@@ -33,7 +35,7 @@ __global__ void Kernel(T* output, int dim) {
 }
 
 template <typename Place>
-void test_pinned_memory() {
+float test_pinned_memory() {
   Place cpu_place;
   paddle::platform::CUDAPlace cuda_place;
 
@@ -133,12 +135,14 @@ void test_pinned_memory() {
     paddle::memory::Free(cpu_place, output_pinned_mem[j]);
     paddle::memory::Free(cuda_place, gpu_mem[j]);
   }
+  return elapsedTime / 30;
 }
 
-TEST(CPUANDCUDAPinned, CPUAllocator) {
-  test_pinned_memory<paddle::platform::CPUPlace>();
+TEST(CPUANDCUDAPinned, CPUAllocatorAndCUDAPinnedAllocator) {
+  // Generally speaking, operation on pinned_memory is faster than that on
+  // unpinned-memory, but if this unit test fails frequently, please close this
+  // test for the time being.
+  float time1 = test_pinned_memory<paddle::platform::CPUPlace>();
+  float time2 = test_pinned_memory<paddle::platform::CUDAPinnedPlace>();
+  EXPECT_GT(time1, time2)
 }
-
-TEST(CPUANDCUDAPinned, CUDAPinnedAllocator) {
-  test_pinned_memory<paddle::platform::CUDAPinnedPlace>();
-}
\ No newline at end of file
-- 
GitLab