From 676690586955d299ae1dad5796b2c7a7e9401e29 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 8 Apr 2020 15:01:22 -0700 Subject: [PATCH 1/5] Performance optimizations for Type Cache type sizes in unused bits from the type ID and rewrite some Type methods to avoid unnecessary calls to `expand` when the type is known to be a basic non-tuple type. This eliminates most of the locking traffic and reduces wall clock time by 52% and CPU time by 73% percent for one real-world program on my machine. --- src/wasm-type.h | 11 +++++-- src/wasm/wasm-type.cpp | 73 +++++++++++++++++++++++++++++------------- src/wasm/wasm.cpp | 8 ++--- 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index e62c59016c6..23143efad13 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -24,7 +24,11 @@ namespace wasm { class Type { - uint32_t id; + // enough for the limit of 1000 function arguments + static constexpr unsigned sizeBits = 10; + static constexpr unsigned unknownSize = (1 << sizeBits) - 1; + unsigned id : 32 - sizeBits; + unsigned _size : sizeBits; void init(const std::vector&); public: @@ -50,16 +54,17 @@ class Type { Type() = default; // ValueType can be implicitly upgraded to Type - constexpr Type(ValueType id) : id(id){}; + constexpr Type(ValueType id) : id(id), _size(id == none ? 0 : 1){}; // But converting raw uint32_t is more dangerous, so make it explicit - constexpr explicit Type(uint32_t id) : id(id){}; + constexpr explicit Type(uint32_t id) : id(id), _size(unknownSize){}; // Construct from lists of elementary types Type(std::initializer_list types); explicit Type(const std::vector& types); // Accessors + size_t size(); size_t size() const; const std::vector& expand() const; diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 3fb783fba26..62ec8b2f8f5 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -92,6 +92,11 @@ void Type::init(const std::vector& types) { } #endif + if (types.size() >= unknownSize) { + WASM_UNREACHABLE("Type too large"); + } + _size = types.size(); + auto lookup = [&]() { auto indexIt = indices.find(types); if (indexIt != indices.end()) { @@ -115,6 +120,9 @@ void Type::init(const std::vector& types) { if (lookup()) { return; } + if (typeLists.size() >= (1 << (32 - sizeBits))) { + WASM_UNREACHABLE("Too many types!"); + } id = typeLists.size(); typeLists.push_back(std::make_unique>(types)); indices[types] = id; @@ -125,7 +133,19 @@ Type::Type(std::initializer_list types) { init(types); } Type::Type(const std::vector& types) { init(types); } -size_t Type::size() const { return expand().size(); } +size_t Type::size() { + if (_size == unknownSize) { + _size = expand().size(); + } + return _size; +} + +size_t Type::size() const { + if (_size == unknownSize) { + return expand().size(); + } + return _size; +} const std::vector& Type::expand() const { std::shared_lock lock(mutex); @@ -146,28 +166,34 @@ bool Type::operator<(const Type& other) const { unsigned Type::getByteSize() const { // TODO: alignment? - unsigned size = 0; - for (auto t : expand()) { + auto getSingleByteSize = [](Type t) { switch (t.getSingle()) { case Type::i32: case Type::f32: - size += 4; - break; + return 4; case Type::i64: case Type::f64: - size += 8; - break; + return 8; case Type::v128: - size += 16; - break; + return 16; case Type::funcref: case Type::anyref: case Type::nullref: case Type::exnref: case Type::none: case Type::unreachable: - WASM_UNREACHABLE("invalid type"); + break; } + WASM_UNREACHABLE("invalid type"); + }; + + if (isSingle()) { + return getSingleByteSize(*this); + } + + unsigned size = 0; + for (auto t : expand()) { + size += getSingleByteSize(t); } return size; } @@ -197,25 +223,26 @@ Type Type::reinterpret() const { } FeatureSet Type::getFeatures() const { - FeatureSet feats = FeatureSet::MVP; - const auto& elements = expand(); - if (elements.size() > 1) { - feats = FeatureSet::Multivalue; - } - for (Type t : elements) { + auto getSingleFeatures = [](Type t) { switch (t.getSingle()) { case Type::v128: - feats |= FeatureSet::SIMD; - break; + return FeatureSet::SIMD; case Type::anyref: - feats |= FeatureSet::ReferenceTypes; - break; + return FeatureSet::ReferenceTypes; case Type::exnref: - feats |= FeatureSet::ExceptionHandling; - break; + return FeatureSet::ExceptionHandling; default: - break; + return FeatureSet::MVP; } + }; + + if (isSingle()) { + return getSingleFeatures(*this); + } + + FeatureSet feats = FeatureSet::Multivalue; + for (Type t : expand()) { + feats |= getSingleFeatures(t); } return feats; } diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp index 6f7ea4f869d..87981729ed1 100644 --- a/src/wasm/wasm.cpp +++ b/src/wasm/wasm.cpp @@ -992,11 +992,11 @@ Index Function::getLocalIndex(Name name) { Index Function::getVarIndexBase() { return sig.params.size(); } Type Function::getLocalType(Index index) { - const std::vector& params = sig.params.expand(); - if (index < params.size()) { - return params[index]; + auto numParams = sig.params.size(); + if (index < numParams) { + return sig.params.expand()[index]; } else if (isVar(index)) { - return vars[index - params.size()]; + return vars[index - numParams]; } else { WASM_UNREACHABLE("invalid local index"); } From 86516e62257575c16d502e7e0c57571f6102ebe8 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 8 Apr 2020 16:25:01 -0700 Subject: [PATCH 2/5] Update constants --- src/wasm-type.h | 11 ++++++----- src/wasm/wasm-type.cpp | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index 23143efad13..b7d8be25b1b 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -25,10 +25,11 @@ namespace wasm { class Type { // enough for the limit of 1000 function arguments - static constexpr unsigned sizeBits = 10; - static constexpr unsigned unknownSize = (1 << sizeBits) - 1; - unsigned id : 32 - sizeBits; - unsigned _size : sizeBits; + static constexpr unsigned SIZE_BITS = 10; + static constexpr unsigned ID_BITS = 32 - SIZE_BITS; + static constexpr unsigned UNKNOWN_SIZE = (1 << SIZE_BITS) - 1; + unsigned id : ID_BITS; + unsigned _size : SIZE_BITS; void init(const std::vector&); public: @@ -57,7 +58,7 @@ class Type { constexpr Type(ValueType id) : id(id), _size(id == none ? 0 : 1){}; // But converting raw uint32_t is more dangerous, so make it explicit - constexpr explicit Type(uint32_t id) : id(id), _size(unknownSize){}; + constexpr explicit Type(uint32_t id) : id(id), _size(UNKNOWN_SIZE){}; // Construct from lists of elementary types Type(std::initializer_list types); diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 62ec8b2f8f5..96bbae9b3e6 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -92,7 +92,7 @@ void Type::init(const std::vector& types) { } #endif - if (types.size() >= unknownSize) { + if (types.size() >= UNKNOWN_SIZE) { WASM_UNREACHABLE("Type too large"); } _size = types.size(); @@ -120,7 +120,7 @@ void Type::init(const std::vector& types) { if (lookup()) { return; } - if (typeLists.size() >= (1 << (32 - sizeBits))) { + if (typeLists.size() >= (1 << (ID_BITS))) { WASM_UNREACHABLE("Too many types!"); } id = typeLists.size(); @@ -134,14 +134,14 @@ Type::Type(std::initializer_list types) { init(types); } Type::Type(const std::vector& types) { init(types); } size_t Type::size() { - if (_size == unknownSize) { + if (_size == UNKNOWN_SIZE) { _size = expand().size(); } return _size; } size_t Type::size() const { - if (_size == unknownSize) { + if (_size == UNKNOWN_SIZE) { return expand().size(); } return _size; From de4a7540352f739e30c4dcd1fe85a5d588f9c722 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 8 Apr 2020 17:21:25 -0700 Subject: [PATCH 3/5] Remove UNKONWN_SIZE --- src/wasm-type.h | 4 +--- src/wasm/wasm-type.cpp | 14 ++++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/wasm-type.h b/src/wasm-type.h index b7d8be25b1b..f3db912d16b 100644 --- a/src/wasm-type.h +++ b/src/wasm-type.h @@ -27,7 +27,6 @@ class Type { // enough for the limit of 1000 function arguments static constexpr unsigned SIZE_BITS = 10; static constexpr unsigned ID_BITS = 32 - SIZE_BITS; - static constexpr unsigned UNKNOWN_SIZE = (1 << SIZE_BITS) - 1; unsigned id : ID_BITS; unsigned _size : SIZE_BITS; void init(const std::vector&); @@ -58,14 +57,13 @@ class Type { constexpr Type(ValueType id) : id(id), _size(id == none ? 0 : 1){}; // But converting raw uint32_t is more dangerous, so make it explicit - constexpr explicit Type(uint32_t id) : id(id), _size(UNKNOWN_SIZE){}; + explicit Type(uint32_t id); // Construct from lists of elementary types Type(std::initializer_list types); explicit Type(const std::vector& types); // Accessors - size_t size(); size_t size() const; const std::vector& expand() const; diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 96bbae9b3e6..6ef2f4d7330 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -92,7 +92,7 @@ void Type::init(const std::vector& types) { } #endif - if (types.size() >= UNKNOWN_SIZE) { + if (types.size() >= (1 << SIZE_BITS)) { WASM_UNREACHABLE("Type too large"); } _size = types.size(); @@ -133,17 +133,15 @@ Type::Type(std::initializer_list types) { init(types); } Type::Type(const std::vector& types) { init(types); } -size_t Type::size() { - if (_size == UNKNOWN_SIZE) { - _size = expand().size(); +Type::Type(uint32_t _id) { + id = _id; + { + std::shared_lock lock(mutex); + _size = typeLists[id]->size(); } - return _size; } size_t Type::size() const { - if (_size == UNKNOWN_SIZE) { - return expand().size(); - } return _size; } From e3ab24527cee2e21c198d1bb7802e9d56cb4a2f9 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Wed, 8 Apr 2020 17:23:49 -0700 Subject: [PATCH 4/5] Remove extra parens --- src/wasm/wasm-type.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index 6ef2f4d7330..b7b382b6a57 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -120,7 +120,7 @@ void Type::init(const std::vector& types) { if (lookup()) { return; } - if (typeLists.size() >= (1 << (ID_BITS))) { + if (typeLists.size() >= (1 << ID_BITS)) { WASM_UNREACHABLE("Too many types!"); } id = typeLists.size(); From 6fb1827ae137de4bd98e50c438ee78c16a9ec841 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 9 Apr 2020 00:47:53 -0700 Subject: [PATCH 5/5] reformat --- src/wasm/wasm-type.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wasm/wasm-type.cpp b/src/wasm/wasm-type.cpp index b7b382b6a57..da7fb50b698 100644 --- a/src/wasm/wasm-type.cpp +++ b/src/wasm/wasm-type.cpp @@ -141,9 +141,7 @@ Type::Type(uint32_t _id) { } } -size_t Type::size() const { - return _size; -} +size_t Type::size() const { return _size; } const std::vector& Type::expand() const { std::shared_lock lock(mutex);