Commit 281dd3dc authored by Adrienne Walker's avatar Adrienne Walker Committed by Commit Bot

cc: Fix some alignment issues in PaintOpWriter

Debug builds of Android were crashing because of misaligned read/writes
of float variables in PaintOpWriter::WriteSimple and the accompanying
reader.

Fix this by rounding up all WriteSimples to align the size they write
to 4 bytes.  This is just a spot fix, but fixes most of the alignment
issues on a few checked pages.  If we wanted to move to DCHECKing
the alignment, this would probably need more work.

This continues to avoid calling AlignMemory in Write/ReadSimple, as that
appears to cause about 30% slowdown in serializing/deserializing speed
in the cc_perftests --gtest_filter=PaintOp* tests.

Bug: 905665
Change-Id: Ib6db27d7cafe22b40b96f841dc2df54c1b6a4f1c
Reviewed-on: https://chromium-review.googlesource.com/c/1343565
Commit-Queue: enne <enne@chromium.org>
Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609968}
parent 93de1767
......@@ -7,6 +7,7 @@
#include <stddef.h>
#include <algorithm>
#include "base/bits.h"
#include "base/stl_util.h"
#include "cc/paint/image_transfer_cache_entry.h"
#include "cc/paint/paint_cache.h"
......@@ -100,7 +101,12 @@ template <typename T>
void PaintOpReader::ReadSimple(T* val) {
static_assert(base::is_trivially_copyable<T>::value,
"Not trivially copyable");
if (remaining_bytes_ < sizeof(T))
// Align everything to 4 bytes, as the writer does.
static constexpr size_t kAlign = 4;
size_t size = base::bits::Align(sizeof(T), kAlign);
if (remaining_bytes_ < size)
SetInvalid();
if (!valid_)
return;
......@@ -111,8 +117,8 @@ void PaintOpReader::ReadSimple(T* val) {
// use assignment.
*val = *reinterpret_cast<const T*>(const_cast<const char*>(memory_));
memory_ += sizeof(T);
remaining_bytes_ -= sizeof(T);
memory_ += size;
remaining_bytes_ -= size;
}
template <typename T>
......
......@@ -4,6 +4,7 @@
#include "cc/paint/paint_op_writer.h"
#include "base/bits.h"
#include "cc/paint/draw_image.h"
#include "cc/paint/image_provider.h"
#include "cc/paint/image_transfer_cache_entry.h"
......@@ -92,16 +93,22 @@ PaintOpWriter::~PaintOpWriter() = default;
template <typename T>
void PaintOpWriter::WriteSimple(const T& val) {
static_assert(base::is_trivially_copyable<T>::value, "");
EnsureBytes(sizeof(T));
// Round up each write to 4 bytes. This is not technically perfect alignment,
// but it is about 30% faster to post-align each write to 4 bytes than it is
// to pre-align memory to the correct alignment.
// TODO(enne): maybe we should do this correctly and DCHECK alignment.
static constexpr size_t kAlign = 4;
size_t size = base::bits::Align(sizeof(T), kAlign);
EnsureBytes(size);
if (!valid_)
return;
reinterpret_cast<T*>(memory_)[0] = val;
memory_ += sizeof(T);
remaining_bytes_ -= sizeof(T);
memory_ += size;
remaining_bytes_ -= size;
}
void PaintOpWriter::WriteFlattenable(const SkFlattenable* val) {
if (!val) {
WriteSize(static_cast<size_t>(0u));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment