Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aa0358b8 authored by Tom Cherry's avatar Tom Cherry Committed by Gerrit Code Review
Browse files

Merge "logd: always compress SerializedLogChunk in FinishWriting()"

parents e866ce04 59caa7a0
Loading
Loading
Loading
Loading
+3 −3
Original line number Original line Diff line number Diff line
@@ -31,7 +31,7 @@ SerializedFlushToState::SerializedFlushToState(uint64_t start, LogMask log_mask)
SerializedFlushToState::~SerializedFlushToState() {
SerializedFlushToState::~SerializedFlushToState() {
    log_id_for_each(i) {
    log_id_for_each(i) {
        if (log_positions_[i]) {
        if (log_positions_[i]) {
            log_positions_[i]->buffer_it->DecReaderRefCount(true);
            log_positions_[i]->buffer_it->DecReaderRefCount();
        }
        }
    }
    }
}
}
@@ -78,7 +78,7 @@ void SerializedFlushToState::AddMinHeapEntry(log_id_t log_id) {
            logs_needed_from_next_position_[log_id] = true;
            logs_needed_from_next_position_[log_id] = true;
        } else {
        } else {
            // Otherwise, if there is another buffer piece, move to that and do the same check.
            // Otherwise, if there is another buffer piece, move to that and do the same check.
            buffer_it->DecReaderRefCount(true);
            buffer_it->DecReaderRefCount();
            ++buffer_it;
            ++buffer_it;
            buffer_it->IncReaderRefCount();
            buffer_it->IncReaderRefCount();
            log_positions_[log_id]->read_offset = 0;
            log_positions_[log_id]->read_offset = 0;
@@ -134,7 +134,7 @@ void SerializedFlushToState::Prune(log_id_t log_id,
    }
    }


    // // Decrease the ref count since we're deleting our reference.
    // // Decrease the ref count since we're deleting our reference.
    buffer_it->DecReaderRefCount(false);
    buffer_it->DecReaderRefCount();


    // Delete in the reference.
    // Delete in the reference.
    log_positions_[log_id].reset();
    log_positions_[log_id].reset();
+1 −1
Original line number Original line Diff line number Diff line
@@ -123,7 +123,7 @@ void SerializedLogBuffer::RemoveChunkFromStats(log_id_t log_id, SerializedLogChu
        stats_->Subtract(entry->ToLogStatisticsElement(log_id));
        stats_->Subtract(entry->ToLogStatisticsElement(log_id));
        read_offset += entry->total_len();
        read_offset += entry->total_len();
    }
    }
    chunk.DecReaderRefCount(false);
    chunk.DecReaderRefCount();
}
}


void SerializedLogBuffer::NotifyReadersOfPrune(
void SerializedLogBuffer::NotifyReadersOfPrune(
+8 −8
Original line number Original line Diff line number Diff line
@@ -31,7 +31,6 @@ void SerializedLogChunk::Compress() {
                  << " size used: " << write_offset_
                  << " size used: " << write_offset_
                  << " compressed size: " << compressed_log_.size();
                  << " compressed size: " << compressed_log_.size();
    }
    }
    contents_.Resize(0);
}
}


// TODO: Develop a better reference counting strategy to guard against the case where the writer is
// TODO: Develop a better reference counting strategy to guard against the case where the writer is
@@ -44,13 +43,13 @@ void SerializedLogChunk::IncReaderRefCount() {
    CompressionEngine::GetInstance().Decompress(compressed_log_, contents_);
    CompressionEngine::GetInstance().Decompress(compressed_log_, contents_);
}
}


void SerializedLogChunk::DecReaderRefCount(bool compress) {
void SerializedLogChunk::DecReaderRefCount() {
    CHECK_NE(reader_ref_count_, 0U);
    CHECK_NE(reader_ref_count_, 0U);
    if (--reader_ref_count_ != 0) {
    if (--reader_ref_count_ != 0) {
        return;
        return;
    }
    }
    if (compress && !writer_active_) {
    if (!writer_active_) {
        Compress();
        contents_.Resize(0);
    }
    }
}
}


@@ -83,18 +82,19 @@ bool SerializedLogChunk::ClearUidLogs(uid_t uid, log_id_t log_id, LogStatistics*
    }
    }


    if (new_write_offset == 0) {
    if (new_write_offset == 0) {
        DecReaderRefCount(false);
        DecReaderRefCount();
        return true;
        return true;
    }
    }


    // Clear the old compressed logs and set write_offset_ appropriately for DecReaderRefCount()
    // Clear the old compressed logs and set write_offset_ appropriately to compress the new
    // to compress the new partially cleared log.
    // partially cleared log.
    if (new_write_offset != write_offset_) {
    if (new_write_offset != write_offset_) {
        compressed_log_.Resize(0);
        compressed_log_.Resize(0);
        write_offset_ = new_write_offset;
        write_offset_ = new_write_offset;
        Compress();
    }
    }


    DecReaderRefCount(true);
    DecReaderRefCount();


    return false;
    return false;
}
}
+3 −4
Original line number Original line Diff line number Diff line
@@ -30,9 +30,7 @@ class SerializedLogChunk {


    void Compress();
    void Compress();
    void IncReaderRefCount();
    void IncReaderRefCount();
    // Decrease the reader ref count and compress the log if appropriate.  `compress` should only be
    void DecReaderRefCount();
    // set to false in the case that the log buffer will be deleted afterwards.
    void DecReaderRefCount(bool compress);


    // Must have no readers referencing this.  Return true if there are no logs left in this chunk.
    // Must have no readers referencing this.  Return true if there are no logs left in this chunk.
    bool ClearUidLogs(uid_t uid, log_id_t log_id, LogStatistics* stats);
    bool ClearUidLogs(uid_t uid, log_id_t log_id, LogStatistics* stats);
@@ -50,8 +48,9 @@ class SerializedLogChunk {


    void FinishWriting() {
    void FinishWriting() {
        writer_active_ = false;
        writer_active_ = false;
        if (reader_ref_count_ == 0) {
        Compress();
        Compress();
        if (reader_ref_count_ == 0) {
            contents_.Resize(0);
        }
        }
    }
    }


+3 −4
Original line number Original line Diff line number Diff line
@@ -113,8 +113,7 @@ TEST(SerializedLogChunk, three_logs) {
TEST(SerializedLogChunk, catch_DecCompressedRef_CHECK) {
TEST(SerializedLogChunk, catch_DecCompressedRef_CHECK) {
    size_t chunk_size = 10 * 4096;
    size_t chunk_size = 10 * 4096;
    auto chunk = SerializedLogChunk{chunk_size};
    auto chunk = SerializedLogChunk{chunk_size};
    EXPECT_DEATH({ chunk.DecReaderRefCount(true); }, "");
    EXPECT_DEATH({ chunk.DecReaderRefCount(); }, "");
    EXPECT_DEATH({ chunk.DecReaderRefCount(false); }, "");
}
}


// Check that the CHECK() in ClearUidLogs() if the ref count is greater than 0 is caught.
// Check that the CHECK() in ClearUidLogs() if the ref count is greater than 0 is caught.
@@ -123,7 +122,7 @@ TEST(SerializedLogChunk, catch_ClearUidLogs_CHECK) {
    auto chunk = SerializedLogChunk{chunk_size};
    auto chunk = SerializedLogChunk{chunk_size};
    chunk.IncReaderRefCount();
    chunk.IncReaderRefCount();
    EXPECT_DEATH({ chunk.ClearUidLogs(1000, LOG_ID_MAIN, nullptr); }, "");
    EXPECT_DEATH({ chunk.ClearUidLogs(1000, LOG_ID_MAIN, nullptr); }, "");
    chunk.DecReaderRefCount(false);
    chunk.DecReaderRefCount();
}
}


class UidClearTest : public testing::TestWithParam<bool> {
class UidClearTest : public testing::TestWithParam<bool> {
@@ -144,7 +143,7 @@ class UidClearTest : public testing::TestWithParam<bool> {
        check(chunk_);
        check(chunk_);


        if (finish_writing) {
        if (finish_writing) {
            chunk_.DecReaderRefCount(false);
            chunk_.DecReaderRefCount();
        }
        }
    }
    }