[skottie] SkTArray begone
SkTArray is not a good choice for tigh allocations:
- its minimum allocation count is 8
- reserve(sz) doesn't adjust capacity to the exact value, but padds
the same way as normal growth (max(sz, 8, 50%))
- no shrink_to_fit() function to trim unneeded capacity
Since keyframed properties with a small number of frames are quite
common in Lottie, this adds significant heap overhead.
Switch to std::vector(), reserve() to the estimated frame count and
shrink_to_fit() when done parsing.
Bug: skia:8340
Change-Id: Id575e2da2fd17537948c2b38485a8accdb9f7a8b
Reviewed-on: https://skia-review.googlesource.com/150905
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
diff --git a/modules/skottie/src/SkottieAnimator.cpp b/modules/skottie/src/SkottieAnimator.cpp
index c5cdf09..463beda 100644
--- a/modules/skottie/src/SkottieAnimator.cpp
+++ b/modules/skottie/src/SkottieAnimator.cpp
@@ -11,9 +11,9 @@
#include "SkottieValue.h"
#include "SkSGScene.h"
#include "SkString.h"
-#include "SkTArray.h"
#include <memory>
+#include <vector>
namespace skottie {
namespace internal {
@@ -30,7 +30,7 @@
class KeyframeAnimatorBase : public sksg::Animator {
public:
- int count() const { return fRecs.count(); }
+ size_t count() const { return fRecs.size(); }
protected:
KeyframeAnimatorBase() = default;
@@ -110,7 +110,7 @@
int cm_idx = -1;
if (c0 != kDefaultC0 || c1 != kDefaultC1) {
// TODO: is it worth de-duping these?
- cm_idx = fCubicMaps.count();
+ cm_idx = SkToInt(fCubicMaps.size());
fCubicMaps.emplace_back();
// TODO: why do we have to plug these inverted?
fCubicMaps.back().setPts(c1, c0);
@@ -124,9 +124,17 @@
fRecs.pop_back();
}
+ fRecs.shrink_to_fit();
+ fCubicMaps.shrink_to_fit();
+
SkASSERT(fRecs.empty() || fRecs.back().isValid());
}
+ void reserve(size_t frame_count) {
+ fRecs.reserve(frame_count);
+ fCubicMaps.reserve(frame_count);
+ }
+
private:
const KeyframeRec* findFrame(float t) const {
SkASSERT(!fRecs.empty());
@@ -165,9 +173,9 @@
return f0;
}
- SkTArray<KeyframeRec> fRecs;
- SkTArray<SkCubicMap> fCubicMaps;
- const KeyframeRec* fCachedRec = nullptr;
+ std::vector<KeyframeRec> fRecs;
+ std::vector<SkCubicMap> fCubicMaps;
+ const KeyframeRec* fCachedRec = nullptr;
using INHERITED = sksg::Animator;
};
@@ -198,7 +206,16 @@
const AnimationBuilder* abuilder,
std::function<void(const T&)>&& apply)
: fApplyFunc(std::move(apply)) {
+ // Generally, each keyframe holds two values (start, end) and a cubic mapper. Except
+ // the last frame, which only holds a marker timestamp. Then, the values series is
+ // contiguous (keyframe[i].end == keyframe[i + 1].start), and we dedupe them.
+ // => we'll store (keyframes.size) values and (keyframe.size - 1) recs and cubic maps.
+ fVs.reserve(jframes.size());
+ this->reserve(SkTMax<size_t>(jframes.size(), 1) - 1);
+
this->parseKeyFrames(jframes, abuilder);
+
+ fVs.shrink_to_fit();
}
int parseValue(const skjson::Value& jv, const AnimationBuilder* abuilder) override {
@@ -212,7 +229,7 @@
if (fVs.empty() || val != fVs.back()) {
fVs.push_back(std::move(val));
}
- return fVs.count() - 1;
+ return SkToInt(fVs.size()) - 1;
}
const T* eval(const KeyframeRec& rec, float t, T* v) const {
@@ -232,7 +249,7 @@
}
const std::function<void(const T&)> fApplyFunc;
- SkTArray<T> fVs;
+ std::vector<T> fVs;
// LERP storage: we use this to temporarily store interpolation results.
// Alternatively, the temp result could live on the stack -- but for vector values that would