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

Commit cae6da6b authored by Leon Scroggins III's avatar Leon Scroggins III
Browse files

[frameworks/base] Make SkiaMemoryTracer::TraceValue own its strings 

TraceValue had an unowned pointer to a const char (string). But there
was no guarantee that the string lived long enough for TraceValue's use.
Switch to an owned string type.

Use std::string, even though SkString has a little bit of use in this
class. Using SkString would've resulted in a map that had both types
of string, and converging on the standard seems better anyway.

Update other unowned pointers which often reference TraceValues to
owned strings as well. Use std::optional instead of a possible null.

Leave some raw strings alone:
- The ResourceMap is only used with static strings
- The virtual method overrides require const char*, but they quickly
  convert to owned strings
- The categoryKey constructor still takes a const char*, but also
  converts to an owned string. Based on the current use, it could
  have remained a raw string, but it is later used with now-owned
  strings.
- `entry` is a short lived const char*, which obviously is safe.

Bug: 305919946
Test: adb shell dumpsys gfxinfo
Change-Id: I30b990771cc7f3ccf2769efc5aafb68c957eca9f
parent f192af2a

libs/hwui/pipeline/skia/SkiaMemoryTracer.cpp

+21 −20
Original line number Original line Diff line number Diff line
@@ -32,13 +32,13 @@ SkiaMemoryTracer::SkiaMemoryTracer(const char* categoryKey, bool itemizeType) 
        , mTotalSize("bytes", 0)

        , mTotalSize("bytes", 0)

        , mPurgeableSize("bytes", 0) {}

        , mPurgeableSize("bytes", 0) {}





const char* SkiaMemoryTracer::mapName(const char* resourceName) {

std::optional<std::string> SkiaMemoryTracer::mapName(const std::string& resourceName) {

    for (auto& resource : mResourceMap) {

    for (auto& resource : mResourceMap) {

        if (SkStrContains(resourceName, resource.first)) {

        if (resourceName.find(resource.first) != std::string::npos) {

            return resource.second;

            return resource.second;

        }

        }

    }

    }

    return nullptr;

    return std::nullopt;

}

}





void SkiaMemoryTracer::processElement() {

void SkiaMemoryTracer::processElement() {
@@ -62,7 +62,7 @@ void SkiaMemoryTracer::processElement() { 
        }

        }





        // find the type if one exists

        // find the type if one exists

        const char* type;

        std::string type;

        auto typeResult = mCurrentValues.find("type");

        auto typeResult = mCurrentValues.find("type");

        if (typeResult != mCurrentValues.end()) {

        if (typeResult != mCurrentValues.end()) {

            type = typeResult->second.units;

            type = typeResult->second.units;
@@ -71,14 +71,13 @@ void SkiaMemoryTracer::processElement() { 
        }

        }





        // compute the type if we are itemizing or use the default "size" if we are not

        // compute the type if we are itemizing or use the default "size" if we are not

        const char* key = (mItemizeType) ? type : sizeResult->first;

        std::string key = (mItemizeType) ? type : sizeResult->first;

        SkASSERT(key != nullptr);





        // compute the top level element name using either the map or category key

        // compute the top level element name using either the map or category key

        const char* resourceName = mapName(mCurrentElement.c_str());

        std::optional<std::string> resourceName = mapName(mCurrentElement);

        if (mCategoryKey != nullptr) {

        if (mCategoryKey) {

            // find the category if one exists

            // find the category if one exists

            auto categoryResult = mCurrentValues.find(mCategoryKey);

            auto categoryResult = mCurrentValues.find(*mCategoryKey);

            if (categoryResult != mCurrentValues.end()) {

            if (categoryResult != mCurrentValues.end()) {

                resourceName = categoryResult->second.units;

                resourceName = categoryResult->second.units;

            } else if (mItemizeType) {

            } else if (mItemizeType) {
@@ -87,11 +86,11 @@ void SkiaMemoryTracer::processElement() { 
        }

        }





        // if we don't have a pretty name then use the dumpName

        // if we don't have a pretty name then use the dumpName

        if (resourceName == nullptr) {

        if (!resourceName) {

            resourceName = mCurrentElement.c_str();

            resourceName = mCurrentElement;

        }

        }





        auto result = mResults.find(resourceName);

        auto result = mResults.find(*resourceName);

        if (result != mResults.end()) {

        if (result != mResults.end()) {

            auto& resourceValues = result->second;

            auto& resourceValues = result->second;

            typeResult = resourceValues.find(key);

            typeResult = resourceValues.find(key);
@@ -106,7 +105,7 @@ void SkiaMemoryTracer::processElement() { 
            TraceValue sizeValue = sizeResult->second;

            TraceValue sizeValue = sizeResult->second;

            mCurrentValues.clear();

            mCurrentValues.clear();

            mCurrentValues.insert({key, sizeValue});

            mCurrentValues.insert({key, sizeValue});

            mResults.insert({resourceName, mCurrentValues});

            mResults.insert({*resourceName, mCurrentValues});

        }

        }

    }

    }
@@ -139,8 +138,9 @@ void SkiaMemoryTracer::logOutput(String8& log) { 
            for (const auto& typedValue : namedItem.second) {

            for (const auto& typedValue : namedItem.second) {

                TraceValue traceValue = convertUnits(typedValue.second);

                TraceValue traceValue = convertUnits(typedValue.second);

                const char* entry = (traceValue.count > 1) ? "entries" : "entry";

                const char* entry = (traceValue.count > 1) ? "entries" : "entry";

                log.appendFormat("    %s: %.2f %s (%d %s)\n", typedValue.first, traceValue.value,

                log.appendFormat("    %s: %.2f %s (%d %s)\n", typedValue.first.c_str(),

                                 traceValue.units, traceValue.count, entry);

                                 traceValue.value, traceValue.units.c_str(), traceValue.count,

                                 entry);

            }

            }

        } else {

        } else {

            auto result = namedItem.second.find("size");

            auto result = namedItem.second.find("size");
@@ -148,7 +148,8 @@ void SkiaMemoryTracer::logOutput(String8& log) { 
                TraceValue traceValue = convertUnits(result->second);

                TraceValue traceValue = convertUnits(result->second);

                const char* entry = (traceValue.count > 1) ? "entries" : "entry";

                const char* entry = (traceValue.count > 1) ? "entries" : "entry";

                log.appendFormat("  %s: %.2f %s (%d %s)\n", namedItem.first.c_str(),

                log.appendFormat("  %s: %.2f %s (%d %s)\n", namedItem.first.c_str(),

                                 traceValue.value, traceValue.units, traceValue.count, entry);

                                 traceValue.value, traceValue.units.c_str(), traceValue.count,

                                 entry);

            }

            }

        }

        }

    }

    }
@@ -156,7 +157,7 @@ void SkiaMemoryTracer::logOutput(String8& log) { 




size_t SkiaMemoryTracer::total() {

size_t SkiaMemoryTracer::total() {

    processElement();

    processElement();

    if (!strcmp("bytes", mTotalSize.units)) {

    if ("bytes" == mTotalSize.units) {

        return mTotalSize.value;

        return mTotalSize.value;

    }

    }

    return 0;

    return 0;
@@ -166,16 +167,16 @@ void SkiaMemoryTracer::logTotals(String8& log) { 
    TraceValue total = convertUnits(mTotalSize);

    TraceValue total = convertUnits(mTotalSize);

    TraceValue purgeable = convertUnits(mPurgeableSize);

    TraceValue purgeable = convertUnits(mPurgeableSize);

    log.appendFormat("  %.0f bytes, %.2f %s (%.2f %s is purgeable)\n", mTotalSize.value,

    log.appendFormat("  %.0f bytes, %.2f %s (%.2f %s is purgeable)\n", mTotalSize.value,

                     total.value, total.units, purgeable.value, purgeable.units);

                     total.value, total.units.c_str(), purgeable.value, purgeable.units.c_str());

}

}





SkiaMemoryTracer::TraceValue SkiaMemoryTracer::convertUnits(const TraceValue& value) {

SkiaMemoryTracer::TraceValue SkiaMemoryTracer::convertUnits(const TraceValue& value) {

    TraceValue output(value);

    TraceValue output(value);

    if (SkString("bytes") == SkString(output.units) && output.value >= 1024) {

    if ("bytes" == output.units && output.value >= 1024) {

        output.value = output.value / 1024.0f;

        output.value = output.value / 1024.0f;

        output.units = "KB";

        output.units = "KB";

    }

    }

    if (SkString("KB") == SkString(output.units) && output.value >= 1024) {

    if ("KB" == output.units && output.value >= 1024) {

        output.value = output.value / 1024.0f;

        output.value = output.value / 1024.0f;

        output.units = "MB";

        output.units = "MB";

    }

    }

libs/hwui/pipeline/skia/SkiaMemoryTracer.h

+8 −7
Original line number Original line Diff line number Diff line
@@ -16,8 +16,9 @@ 




#pragma once

#pragma once





#include <SkString.h>

#include <SkTraceMemoryDump.h>

#include <SkTraceMemoryDump.h>

#include <optional>

#include <string>

#include <utils/String8.h>

#include <utils/String8.h>

#include <unordered_map>

#include <unordered_map>

#include <vector>

#include <vector>
@@ -60,17 +61,17 @@ private: 
        TraceValue(const char* units, uint64_t value) : units(units), value(value), count(1) {}

        TraceValue(const char* units, uint64_t value) : units(units), value(value), count(1) {}

        TraceValue(const TraceValue& v) : units(v.units), value(v.value), count(v.count) {}

        TraceValue(const TraceValue& v) : units(v.units), value(v.value), count(v.count) {}





        const char* units;

        std::string units;

        float value;

        float value;

        int count;

        int count;

    };

    };





    const char* mapName(const char* resourceName);

    std::optional<std::string> mapName(const std::string& resourceName);

    void processElement();

    void processElement();

    TraceValue convertUnits(const TraceValue& value);

    TraceValue convertUnits(const TraceValue& value);





    const std::vector<ResourcePair> mResourceMap;

    const std::vector<ResourcePair> mResourceMap;

    const char* mCategoryKey = nullptr;

    std::optional<std::string> mCategoryKey;

    const bool mItemizeType;

    const bool mItemizeType;





    // variables storing the size of all elements being dumped

    // variables storing the size of all elements being dumped
@@ -79,10 +80,10 @@ private: 




    // variables storing information on the current node being dumped

    // variables storing information on the current node being dumped

    std::string mCurrentElement;

    std::string mCurrentElement;

    std::unordered_map<const char*, TraceValue> mCurrentValues;

    std::unordered_map<std::string, TraceValue> mCurrentValues;





    // variable that stores the final format of the data after the individual elements are processed

    // variable that stores the final format of the data after the individual elements are processed

    std::unordered_map<std::string, std::unordered_map<const char*, TraceValue>> mResults;

    std::unordered_map<std::string, std::unordered_map<std::string, TraceValue>> mResults;

};

};





} /* namespace skiapipeline */

} /* namespace skiapipeline */