From e2896f46325cf107f8331d3aac1703f84362836d Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Tue, 6 Nov 2018 12:03:17 -0800
Subject: [PATCH] Set cache size to 64 bytes on all platforms

We used to assume 32-bytes cache lines when running on ARM 32-bit mode,
however, that was wrong because 32-bit mode doesn't change the cpu's
cache line. On all modern platforms we support, the cache line
is 64 bytes.

Set Job storage to 48 bytes on all platforms.
---
 filament/src/Froxelizer.cpp             |  4 ++--
 libs/utils/include/utils/JobSystem.h    | 30 +++++++++++++------------
 libs/utils/include/utils/architecture.h |  6 -----
 3 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/filament/src/Froxelizer.cpp b/filament/src/Froxelizer.cpp
index 2c9605c635..9c3069754a 100644
--- a/filament/src/Froxelizer.cpp
+++ b/filament/src/Froxelizer.cpp
@@ -167,12 +167,12 @@ bool Froxelizer::prepare(
 
     // froxel buffer (~32 KiB)
     mFroxelBufferUser = {
-            driverApi.allocatePod<FroxelEntry>(FROXEL_BUFFER_ENTRY_COUNT_MAX, CACHELINE_SIZE),
+            driverApi.allocatePod<FroxelEntry>(FROXEL_BUFFER_ENTRY_COUNT_MAX),
             FROXEL_BUFFER_ENTRY_COUNT_MAX };
 
     // record buffer (~64 KiB)
     mRecordBufferUser = {
-            driverApi.allocatePod<RecordBufferType>(RECORD_BUFFER_ENTRY_COUNT, CACHELINE_SIZE),
+            driverApi.allocatePod<RecordBufferType>(RECORD_BUFFER_ENTRY_COUNT),
             RECORD_BUFFER_ENTRY_COUNT };
 
     /*
diff --git a/libs/utils/include/utils/JobSystem.h b/libs/utils/include/utils/JobSystem.h
index 50a0d30488..c0b1c429e5 100644
--- a/libs/utils/include/utils/JobSystem.h
+++ b/libs/utils/include/utils/JobSystem.h
@@ -46,29 +46,31 @@ public:
 
     using JobFunc = void(*)(void*, JobSystem&, Job*);
 
-    class alignas(CACHELINE_SIZE) Job { // NOLINT(cppcoreguidelines-pro-type-member-init)
+    class alignas(CACHELINE_SIZE) Job {
     public:
-        Job() noexcept {} // = default;
+        Job() noexcept {} /* = default; */ /* clang bug */ // NOLINT(modernize-use-equals-default,cppcoreguidelines-pro-type-member-init)
         Job(const Job&) = delete;
         Job(Job&&) = delete;
 
-        void* getData() { return storage; }
-        void const* getData() const { return storage; }
     private:
         friend class JobSystem;
 
-        // Size is chosen so that we can store at least std::function<>, the alignas() qualifier
-        // ensures we're multiple of a cache-line.
-        static constexpr size_t JOB_STORAGE_SIZE = (sizeof(std::function<void()>) + sizeof(void*) - 1) / sizeof(void*);
+        // Size is chosen so that we can store at least std::function<>
+        // the alignas() qualifier ensures we're multiple of a cache-line.
+        static constexpr size_t JOB_STORAGE_SIZE = // NOLINT(cert-err58-cpp)
+                (std::max(sizeof(std::function<void()>), size_t(48)) + sizeof(void*) - 1)
+                / sizeof(void*);
 
         // keep it first, so it's correctly aligned with all architectures
-        // this is were we store the job's data, typically a std::function
-        void* storage[JOB_STORAGE_SIZE];
-
-        JobFunc function;
-        uint16_t parent;
-        std::atomic<uint16_t> runningJobCount = { 1 };
-        mutable std::atomic<uint16_t> refCount = { 1 };
+        // this is were we store the job's data, typically a std::function<>
+                                                                // v7 | v8
+        void* storage[JOB_STORAGE_SIZE];                        // 48 | 48
+        JobFunc function;                                       //  4 |  8
+        uint16_t parent;                                        //  2 |  2
+        std::atomic<uint16_t> runningJobCount = { 1 };          //  2 |  2
+        mutable std::atomic<uint16_t> refCount = { 1 };         //  2 |  2
+                                                                //  6 |  2 (padding)
+                                                                // 64 | 64
     };
 
     explicit JobSystem(size_t threadCount = 0, size_t adoptableThreadsCount = 1) noexcept;
diff --git a/libs/utils/include/utils/architecture.h b/libs/utils/include/utils/architecture.h
index f9e029b866..83b1179406 100644
--- a/libs/utils/include/utils/architecture.h
+++ b/libs/utils/include/utils/architecture.h
@@ -21,13 +21,7 @@
 
 namespace utils {
 
-#ifdef __ARM_32BIT_STATE
-// on ARM 32-bits, assume 32-bytes cache lines
-constexpr size_t CACHELINE_SIZE = 32;
-#else
-// on ARM64 and x86 we assume 64-bytes cache lines
 constexpr size_t CACHELINE_SIZE = 64;
-#endif
 
 } // namespace utils