Class/struct layout optimization for blink Resource related classes

third_party/WebKit/Source/platform/loader/fetch/Resource.h

Index: third_party/WebKit/Source/platform/loader/fetch/Resource.h
diff --git a/third_party/WebKit/Source/platform/loader/fetch/Resource.h b/third_party/WebKit/Source/platform/loader/fetch/Resource.h
index 537d295485a5693ab14c937d77bd04f72eb61a1c..9957e5ab8b6f6b12d2049d25c239b065fbeb2a68 100644
--- a/third_party/WebKit/Source/platform/loader/fetch/Resource.h
+++ b/third_party/WebKit/Source/platform/loader/fetch/Resource.h
@@ -69,7 +69,7 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
   // |Type| enum values are used in UMAs, so do not change the values of
   // existing |Type|. When adding a new |Type|, append it at the end and update
   // |kLastResourceType|.
-  enum Type {
+  enum Type : uint8_t {
     kMainResource,
     kImage,
     kCSSStyleSheet,
@@ -157,7 +157,7 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
   void AddClient(ResourceClient*, PreloadReferencePolicy = kMarkAsReferenced);
   void RemoveClient(ResourceClient*);
 
-  enum PreloadResult {
+  enum PreloadResult : uint8_t {
     kPreloadNotReferenced,
     kPreloadReferenced,
     kPreloadReferencedWhileLoading,
@@ -429,6 +429,20 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
   // MemoryCoordinatorClient overrides:
   void OnPurgeMemory() override;
 
+  PreloadResult preload_result_;
+  Type type_;
+  ResourceStatus status_;
+
+  bool needs_synchronous_cache_hit_ : 1;

[dcheng, 2017/06/05 18:15:40]

Is the bitfield packing necessary? We don't pack everything as bitfields, so
it's not clear why some objects are packed and others aren't.

[stanisc, 2017/06/06 22:47:55]

Not necessary in this particular case. I was debating whether to introduce it or
not and decided to introduce it to save another 16 bytes of the class sizeof
(the actual difference would be 8 bytes but the allocation granularity is 16
bytes). 
Bitfields can't be applied to the last two boolean fields because there is some
code that takes address of those field.

I am OK with reverting these back to regular boolean fields if you think this is
sketchy.

[dcheng, 2017/06/07 17:09:22]

If I had written the CL, I probably wouldn't have used bitfields for
consistency, but I don't feel strongly about it.

[stanisc, 2017/06/08 21:34:03]

OK, I've removed bitfields because that has been suggested by another reviewer
too.

+  bool link_preload_ : 1;
+  bool is_revalidating_ : 1;
+  bool is_alive_ : 1;
+
+  bool is_add_remove_client_prohibited_;
+  bool is_revalidation_start_forbidden_ = false;
+
+  ResourceIntegrityDisposition integrity_disposition_;

[dcheng, 2017/06/07 17:09:22]

This IMO is the big disadvantage of reordering fields to optimize class layout:
this was originally next to IntegrityMetadataSet, which makes sense--but now
it's pretty far away. Anything we can do to put it nearby without wasting too
much padding?

[stanisc, 2017/06/08 21:34:03]

OK, moved it back.

+
   Member<CachedMetadataHandlerImpl> cache_handler_;
   RefPtr<SecurityOrigin> fetcher_security_origin_;
 
@@ -438,6 +452,9 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
 
   unsigned long identifier_;
 
+  unsigned preload_count_;
+  double preload_discovery_time_;
+
   size_t encoded_size_;
   size_t encoded_size_memory_usage_;
   size_t decoded_size_;
@@ -448,24 +465,8 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
   // and thus potential bugs. crbug.com/594644
   const size_t overhead_size_;
 
-  unsigned preload_count_;
-
-  double preload_discovery_time_;
-
   String cache_identifier_;
 
-  PreloadResult preload_result_;
-  Type type_;
-  ResourceStatus status_;
-
-  bool needs_synchronous_cache_hit_;
-  bool link_preload_;
-  bool is_revalidating_;
-  bool is_alive_;
-  bool is_add_remove_client_prohibited_;
-  bool is_revalidation_start_forbidden_ = false;
-
-  ResourceIntegrityDisposition integrity_disposition_;
   IntegrityMetadataSet integrity_metadata_;
 
   // Ordered list of all redirects followed while fetching this resource.