Skip to content

Commit b6cb992

Browse files
author
Tom Cherry
committed
logd: replace std::vector<uint8_t> in SerializedLogChunk
Turns out std::vector::resize() and std::vector::clear() don't actually deallocate any memory. std::vector::shrink_to_fit() can be used for this but isn't a 'guarantee'. Instead of trying to get std::vector to play nice, this change replaces std::vector<uint8_t> with std::unique_ptr<uint8_t[]>, which is more accurate to how I'm using this memory anyway. Test: logging unit tests Change-Id: I9638e90bbf50bcf316c5aa172c8278ea945d27e7
1 parent b07e339 commit b6cb992

File tree

5 files changed

+98
-45
lines changed

5 files changed

+98
-45
lines changed

logd/CompressionEngine.cpp

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ CompressionEngine& CompressionEngine::GetInstance() {
2727
return *engine;
2828
}
2929

30-
bool ZlibCompressionEngine::Compress(std::span<uint8_t> in, std::vector<uint8_t>& out) {
30+
bool ZlibCompressionEngine::Compress(SerializedData& in, size_t data_length, SerializedData& out) {
3131
z_stream strm;
3232
strm.zalloc = Z_NULL;
3333
strm.zfree = Z_NULL;
@@ -37,34 +37,34 @@ bool ZlibCompressionEngine::Compress(std::span<uint8_t> in, std::vector<uint8_t>
3737
LOG(FATAL) << "deflateInit() failed";
3838
}
3939

40-
CHECK_LE(in.size(), static_cast<int64_t>(std::numeric_limits<uint32_t>::max()));
41-
uint32_t out_size = deflateBound(&strm, in.size());
40+
CHECK_LE(data_length, in.size());
41+
CHECK_LE(in.size(), std::numeric_limits<uint32_t>::max());
42+
uint32_t deflate_bound = deflateBound(&strm, in.size());
4243

43-
out.resize(out_size);
44-
strm.avail_in = in.size();
45-
strm.next_in = const_cast<uint8_t*>(in.data());
46-
strm.avail_out = out_size;
44+
out.Resize(deflate_bound);
45+
46+
strm.avail_in = data_length;
47+
strm.next_in = in.data();
48+
strm.avail_out = out.size();
4749
strm.next_out = out.data();
4850
ret = deflate(&strm, Z_FINISH);
4951
CHECK_EQ(ret, Z_STREAM_END);
5052

51-
uint32_t compressed_data_size = strm.total_out;
53+
uint32_t compressed_size = strm.total_out;
5254
deflateEnd(&strm);
53-
out.resize(compressed_data_size);
55+
56+
out.Resize(compressed_size);
5457

5558
return true;
5659
}
5760

58-
bool ZlibCompressionEngine::Decompress(const std::vector<uint8_t>& in, std::vector<uint8_t>& out,
59-
size_t out_size) {
60-
out.resize(out_size);
61-
61+
bool ZlibCompressionEngine::Decompress(SerializedData& in, SerializedData& out) {
6262
z_stream strm;
6363
strm.zalloc = Z_NULL;
6464
strm.zfree = Z_NULL;
6565
strm.opaque = Z_NULL;
6666
strm.avail_in = in.size();
67-
strm.next_in = const_cast<uint8_t*>(in.data());
67+
strm.next_in = in.data();
6868
strm.avail_out = out.size();
6969
strm.next_out = out.data();
7070

@@ -79,22 +79,22 @@ bool ZlibCompressionEngine::Decompress(const std::vector<uint8_t>& in, std::vect
7979
return true;
8080
}
8181

82-
bool ZstdCompressionEngine::Compress(std::span<uint8_t> in, std::vector<uint8_t>& out) {
83-
size_t out_size = ZSTD_compressBound(in.size());
84-
out.resize(out_size);
82+
bool ZstdCompressionEngine::Compress(SerializedData& in, size_t data_length, SerializedData& out) {
83+
CHECK_LE(data_length, in.size());
84+
85+
size_t compress_bound = ZSTD_compressBound(data_length);
86+
out.Resize(compress_bound);
8587

86-
out_size = ZSTD_compress(out.data(), out_size, in.data(), in.size(), 1);
88+
size_t out_size = ZSTD_compress(out.data(), out.size(), in.data(), data_length, 1);
8789
if (ZSTD_isError(out_size)) {
8890
LOG(FATAL) << "ZSTD_compress failed: " << ZSTD_getErrorName(out_size);
8991
}
90-
out.resize(out_size);
92+
out.Resize(out_size);
9193

9294
return true;
9395
}
9496

95-
bool ZstdCompressionEngine::Decompress(const std::vector<uint8_t>& in, std::vector<uint8_t>& out,
96-
size_t out_size) {
97-
out.resize(out_size);
97+
bool ZstdCompressionEngine::Decompress(SerializedData& in, SerializedData& out) {
9898
size_t result = ZSTD_decompress(out.data(), out.size(), in.data(), in.size());
9999
if (ZSTD_isError(result)) {
100100
LOG(FATAL) << "ZSTD_decompress failed: " << ZSTD_getErrorName(result);

logd/CompressionEngine.h

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,32 +16,30 @@
1616

1717
#pragma once
1818

19-
#include <span>
20-
#include <vector>
19+
#include <memory>
20+
21+
#include "SerializedData.h"
2122

2223
class CompressionEngine {
2324
public:
2425
static CompressionEngine& GetInstance();
2526

2627
virtual ~CompressionEngine(){};
2728

28-
virtual bool Compress(std::span<uint8_t> in, std::vector<uint8_t>& out) = 0;
29-
// Decompress the contents of `in` into `out`. `out_size` must be set to the decompressed size
30-
// of the contents.
31-
virtual bool Decompress(const std::vector<uint8_t>& in, std::vector<uint8_t>& out,
32-
size_t out_size) = 0;
29+
virtual bool Compress(SerializedData& in, size_t data_length, SerializedData& out) = 0;
30+
// Decompress the contents of `in` into `out`. `out.size()` must be set to the decompressed
31+
// size of the contents.
32+
virtual bool Decompress(SerializedData& in, SerializedData& out) = 0;
3333
};
3434

3535
class ZlibCompressionEngine : public CompressionEngine {
3636
public:
37-
bool Compress(std::span<uint8_t> in, std::vector<uint8_t>& out) override;
38-
bool Decompress(const std::vector<uint8_t>& in, std::vector<uint8_t>& out,
39-
size_t out_size) override;
37+
bool Compress(SerializedData& in, size_t data_length, SerializedData& out) override;
38+
bool Decompress(SerializedData& in, SerializedData& out) override;
4039
};
4140

4241
class ZstdCompressionEngine : public CompressionEngine {
4342
public:
44-
bool Compress(std::span<uint8_t> in, std::vector<uint8_t>& out) override;
45-
bool Decompress(const std::vector<uint8_t>& in, std::vector<uint8_t>& out,
46-
size_t out_size) override;
43+
bool Compress(SerializedData& in, size_t data_length, SerializedData& out) override;
44+
bool Decompress(SerializedData& in, SerializedData& out) override;
4745
};

logd/SerializedData.h

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright (C) 2020 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#pragma once
18+
19+
#include <sys/types.h>
20+
21+
#include <algorithm>
22+
#include <memory>
23+
24+
// This class is used instead of std::string or std::vector because their clear(), erase(), etc
25+
// functions don't actually deallocate. shrink_to_fit() does deallocate but is not guaranteed to
26+
// work and swapping with an empty string/vector is clunky.
27+
class SerializedData {
28+
public:
29+
SerializedData() {}
30+
SerializedData(size_t size) : data_(new uint8_t[size]), size_(size) {}
31+
32+
void Resize(size_t new_size) {
33+
if (size_ == 0) {
34+
data_.reset(new uint8_t[new_size]);
35+
size_ = new_size;
36+
} else if (new_size == 0) {
37+
data_.reset();
38+
size_ = 0;
39+
} else if (new_size != size_) {
40+
std::unique_ptr<uint8_t[]> new_data(new uint8_t[new_size]);
41+
size_t copy_size = std::min(size_, new_size);
42+
memcpy(new_data.get(), data_.get(), copy_size);
43+
data_.swap(new_data);
44+
size_ = new_size;
45+
}
46+
}
47+
48+
uint8_t* data() { return data_.get(); }
49+
const uint8_t* data() const { return data_.get(); }
50+
size_t size() const { return size_; }
51+
52+
private:
53+
std::unique_ptr<uint8_t[]> data_;
54+
size_t size_ = 0;
55+
};

logd/SerializedLogChunk.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,13 @@ SerializedLogChunk::~SerializedLogChunk() {
2525
}
2626

2727
void SerializedLogChunk::Compress() {
28-
if (compressed_log_.empty()) {
29-
CompressionEngine::GetInstance().Compress({contents_.data(), write_offset_},
30-
compressed_log_);
28+
if (compressed_log_.size() == 0) {
29+
CompressionEngine::GetInstance().Compress(contents_, write_offset_, compressed_log_);
3130
LOG(INFO) << "Compressed Log, buffer max size: " << contents_.size()
3231
<< " size used: " << write_offset_
3332
<< " compressed size: " << compressed_log_.size();
3433
}
35-
contents_.resize(0);
34+
contents_.Resize(0);
3635
}
3736

3837
// TODO: Develop a better reference counting strategy to guard against the case where the writer is
@@ -41,7 +40,8 @@ void SerializedLogChunk::IncReaderRefCount() {
4140
if (++reader_ref_count_ != 1 || writer_active_) {
4241
return;
4342
}
44-
CompressionEngine::GetInstance().Decompress(compressed_log_, contents_, write_offset_);
43+
contents_.Resize(write_offset_);
44+
CompressionEngine::GetInstance().Decompress(compressed_log_, contents_);
4545
}
4646

4747
void SerializedLogChunk::DecReaderRefCount(bool compress) {
@@ -90,7 +90,7 @@ bool SerializedLogChunk::ClearUidLogs(uid_t uid, log_id_t log_id, LogStatistics*
9090
// Clear the old compressed logs and set write_offset_ appropriately for DecReaderRefCount()
9191
// to compress the new partially cleared log.
9292
if (new_write_offset != write_offset_) {
93-
compressed_log_.clear();
93+
compressed_log_.Resize(0);
9494
write_offset_ = new_write_offset;
9595
}
9696

logd/SerializedLogChunk.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@
1818

1919
#include <sys/types.h>
2020

21-
#include <vector>
22-
2321
#include "LogWriter.h"
22+
#include "SerializedData.h"
2423
#include "SerializedLogEntry.h"
2524

2625
class SerializedLogChunk {
2726
public:
2827
explicit SerializedLogChunk(size_t size) : contents_(size) {}
28+
SerializedLogChunk(SerializedLogChunk&& other) noexcept = default;
2929
~SerializedLogChunk();
3030

3131
void Compress();
@@ -66,10 +66,10 @@ class SerializedLogChunk {
6666
private:
6767
// The decompressed contents of this log buffer. Deallocated when the ref_count reaches 0 and
6868
// writer_active_ is false.
69-
std::vector<uint8_t> contents_;
69+
SerializedData contents_;
7070
int write_offset_ = 0;
7171
uint32_t reader_ref_count_ = 0;
7272
bool writer_active_ = true;
7373
uint64_t highest_sequence_number_ = 1;
74-
std::vector<uint8_t> compressed_log_;
74+
SerializedData compressed_log_;
7575
};

0 commit comments

Comments
 (0)