oop : Only adjust serialization alignment when needed

Alignment code was showing up in paint op serialization perf, so this
patch makes alignment explicit instead of implicit.  Callers that
know that their writes are already aligned aren't forced into extra
work.

DrawRectOp serialization is 6.2x faster and deserialization is 2.2x faster.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I5678c0510c3200eff496c07a6fbb88e70054b27b
Reviewed-on: https://chromium-review.googlesource.com/798470
Commit-Queue: enne <enne@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520990}

cc/paint/paint_op_buffer.cc

@@ -347,6 +347,7 @@ size_t DrawImageOp::Serialize(const PaintOp* base_op,
     serialized_flags = &op->flags;
   helper.Write(*serialized_flags);
   helper.Write(op->image, options.image_provider);
+  helper.AlignMemory(alignof(SkScalar));
   helper.Write(op->left);
   helper.Write(op->top);
   return helper.size();
@@ -393,6 +394,7 @@ size_t DrawLineOp::Serialize(const PaintOp* base_op,
   if (!serialized_flags)
     serialized_flags = &op->flags;
   helper.Write(*serialized_flags);
+  helper.AlignMemory(alignof(SkScalar));
   helper.Write(op->x0);
   helper.Write(op->y0);
   helper.Write(op->x1);
@@ -476,6 +478,7 @@ size_t DrawTextBlobOp::Serialize(const PaintOp* base_op,
   if (!serialized_flags)
     serialized_flags = &op->flags;
   helper.Write(*serialized_flags);
+  helper.AlignMemory(alignof(SkScalar));
   helper.Write(op->x);
   helper.Write(op->y);
   helper.Write(op->blob);
@@ -694,6 +697,7 @@ PaintOp* DrawImageOp::Deserialize(const volatile void* input,
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
   helper.Read(&op->image);
+  helper.AlignMemory(alignof(SkScalar));
   helper.Read(&op->left);
   helper.Read(&op->top);
   if (!helper.valid() || !op->IsValid()) {
@@ -755,6 +759,7 @@ PaintOp* DrawLineOp::Deserialize(const volatile void* input,
 
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
+  helper.AlignMemory(alignof(SkScalar));
   helper.Read(&op->x0);
   helper.Read(&op->y0);
   helper.Read(&op->x1);
@@ -863,6 +868,7 @@ PaintOp* DrawTextBlobOp::Deserialize(const volatile void* input,
 
   PaintOpReader helper(input, input_size);
   helper.Read(&op->flags);
+  helper.AlignMemory(alignof(SkScalar));
   helper.Read(&op->x);
   helper.Read(&op->y);
   helper.Read(&op->blob);
@@ -1669,8 +1675,8 @@ size_t PaintOp::Serialize(void* memory,
   if (written < 4)
     return 0u;
 
-  size_t aligned_written =
-      MathUtil::UncheckedRoundUp(written, PaintOpBuffer::PaintOpAlign);
+  size_t aligned_written = ((written + PaintOpBuffer::PaintOpAlign - 1) &
+                            ~(PaintOpBuffer::PaintOpAlign - 1));
   if (aligned_written >= kMaxSkip)
     return 0u;
   if (aligned_written > size)

cc/paint/paint_op_reader.cc

@@ -94,8 +94,6 @@ template <typename T>
 void PaintOpReader::ReadSimple(T* val) {
   static_assert(base::is_trivially_copyable<T>::value,
                 "Not trivially copyable");
-  if (!AlignMemory(alignof(T)))
-    SetInvalid();
   if (remaining_bytes_ < sizeof(T))
     SetInvalid();
   if (!valid_)
@@ -117,8 +115,6 @@ void PaintOpReader::ReadFlattenable(sk_sp<T>* val) {
   ReadSimple(&bytes);
   if (remaining_bytes_ < bytes)
     SetInvalid();
-  if (!SkIsAlign4(reinterpret_cast<uintptr_t>(memory_)))
-    SetInvalid();
   if (!valid_)
     return;
   if (bytes == 0)
@@ -127,6 +123,7 @@ void PaintOpReader::ReadFlattenable(sk_sp<T>* val) {
   // This is assumed safe from TOCTOU violations as the flattenable
   // deserializing function uses an SkReadBuffer which reads each piece of
   // memory once much like PaintOpReader does.
+  DCHECK(SkIsAlign4(reinterpret_cast<uintptr_t>(memory_)));
   val->reset(static_cast<T*>(SkValidatingDeserializeFlattenable(
       const_cast<const char*>(memory_), bytes, T::GetFlattenableType())));
   if (!val)
@@ -232,8 +229,11 @@ void PaintOpReader::Read(PaintFlags* flags) {
   // Flattenables must be read at 4-byte boundary, which should be the case
   // here.
   ReadFlattenable(&flags->path_effect_);
+  AlignMemory(4);
   ReadFlattenable(&flags->mask_filter_);
+  AlignMemory(4);
   ReadFlattenable(&flags->color_filter_);
+  AlignMemory(4);
   ReadFlattenable(&flags->draw_looper_);
 
   Read(&flags->shader_);
@@ -490,7 +490,7 @@ void PaintOpReader::Read(SkColorType* color_type) {
   *color_type = static_cast<SkColorType>(raw_color_type);
 }
 
-bool PaintOpReader::AlignMemory(size_t alignment) {
+void PaintOpReader::AlignMemory(size_t alignment) {
   // Due to the math below, alignment must be a power of two.
   DCHECK_GT(alignment, 0u);
   DCHECK_EQ(alignment & (alignment - 1), 0u);
@@ -502,11 +502,10 @@ bool PaintOpReader::AlignMemory(size_t alignment) {
   // however, since it can be slow.
   size_t padding = ((memory + alignment - 1) & ~(alignment - 1)) - memory;
   if (padding > remaining_bytes_)
-    return false;
+    valid_ = false;
 
   memory_ += padding;
   remaining_bytes_ -= padding;
-  return true;
 }
 
 inline void PaintOpReader::SetInvalid() {

cc/paint/paint_op_reader.h

@@ -82,6 +82,9 @@ class CC_PAINT_EXPORT PaintOpReader {
   // would exceed the available budfer.
   const volatile void* ExtractReadableMemory(size_t bytes);
 
+  // Aligns the memory to the given alignment.
+  void AlignMemory(size_t alignment);
+
  private:
   template <typename T>
   void ReadSimple(T* val);
@@ -95,10 +98,6 @@ class CC_PAINT_EXPORT PaintOpReader {
 
   void SetInvalid();
 
-  // Attempts to align the memory to the given alignment. Returns false if there
-  // is unsufficient bytes remaining to do this padding.
-  bool AlignMemory(size_t alignment);
-
   const volatile char* memory_ = nullptr;
   size_t remaining_bytes_ = 0u;
   bool valid_ = true;

cc/paint/paint_op_writer.cc

@@ -23,8 +23,6 @@ PaintOpWriter::~PaintOpWriter() = default;
 template <typename T>
 void PaintOpWriter::WriteSimple(const T& val) {
   static_assert(base::is_trivially_copyable<T>::value, "");
-  if (!AlignMemory(alignof(T)))
-    valid_ = false;
   if (remaining_bytes_ < sizeof(T))
     valid_ = false;
   if (!valid_)
@@ -37,13 +35,14 @@ void PaintOpWriter::WriteSimple(const T& val) {
 }
 
 void PaintOpWriter::WriteFlattenable(const SkFlattenable* val) {
+  DCHECK(SkIsAlign4(reinterpret_cast<uintptr_t>(memory_)))
+      << "Flattenable must start writing at 4 byte alignment.";
+
   if (!val) {
     WriteSize(static_cast<size_t>(0u));
     return;
   }
 
-  DCHECK(SkIsAlign4(reinterpret_cast<uintptr_t>(memory_)))
-      << "Flattenable must start writing at 4 byte alignment.";
   // TODO(enne): change skia API to make this a const parameter.
   sk_sp<SkData> data(
       SkValidatingSerializeFlattenable(const_cast<SkFlattenable*>(val)));
@@ -114,8 +113,11 @@ void PaintOpWriter::Write(const PaintFlags& flags) {
   // Flattenables must be written starting at a 4 byte boundary, which should be
   // the case here.
   WriteFlattenable(flags.path_effect_.get());
+  AlignMemory(4);
   WriteFlattenable(flags.mask_filter_.get());
+  AlignMemory(4);
   WriteFlattenable(flags.color_filter_.get());
+  AlignMemory(4);
   WriteFlattenable(flags.draw_looper_.get());
 
   Write(flags.shader_.get());
@@ -260,7 +262,7 @@ void PaintOpWriter::WriteArray(size_t count, const SkPoint* input) {
   WriteData(bytes, input);
 }
 
-bool PaintOpWriter::AlignMemory(size_t alignment) {
+void PaintOpWriter::AlignMemory(size_t alignment) {
   // Due to the math below, alignment must be a power of two.
   DCHECK_GT(alignment, 0u);
   DCHECK_EQ(alignment & (alignment - 1), 0u);
@@ -272,11 +274,10 @@ bool PaintOpWriter::AlignMemory(size_t alignment) {
   // however, since it can be slow.
   size_t padding = ((memory + alignment - 1) & ~(alignment - 1)) - memory;
   if (padding > remaining_bytes_)
-    return false;
+    valid_ = false;
 
   memory_ += padding;
   remaining_bytes_ -= padding;
-  return true;
 }
 
 }  // namespace cc

cc/paint/paint_op_writer.h

@@ -61,6 +61,9 @@ class CC_PAINT_EXPORT PaintOpWriter {
   }
   void Write(bool data) { Write(static_cast<uint8_t>(data)); }
 
+  // Aligns the memory to the given alignment.
+  void AlignMemory(size_t alignment);
+
  private:
   template <typename T>
   void WriteSimple(const T& val);
@@ -71,10 +74,6 @@ class CC_PAINT_EXPORT PaintOpWriter {
 
   static void TypefaceCataloger(SkTypeface* typeface, void* ctx);
 
-  // Attempts to align the memory to the given alignment. Returns false if there
-  // is unsufficient bytes remaining to do this padding.
-  bool AlignMemory(size_t alignment);
-
   char* memory_ = nullptr;
   size_t size_ = 0u;
   size_t remaining_bytes_ = 0u;