-
Notifications
You must be signed in to change notification settings - Fork 818
Performance optimizations for Type #2733
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,6 +92,11 @@ void Type::init(const std::vector<Type>& types) { | |
} | ||
#endif | ||
|
||
if (types.size() >= UNKNOWN_SIZE) { | ||
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<Type>& types) { | |
if (lookup()) { | ||
return; | ||
} | ||
if (typeLists.size() >= (1 << (ID_BITS))) { | ||
WASM_UNREACHABLE("Too many types!"); | ||
} | ||
id = typeLists.size(); | ||
typeLists.push_back(std::make_unique<std::vector<Type>>(types)); | ||
indices[types] = id; | ||
|
@@ -125,7 +133,19 @@ Type::Type(std::initializer_list<Type> types) { init(types); } | |
|
||
Type::Type(const std::vector<Type>& types) { init(types); } | ||
|
||
size_t Type::size() const { return expand().size(); } | ||
size_t Type::size() { | ||
if (_size == UNKNOWN_SIZE) { | ||
_size = expand().size(); | ||
} | ||
return _size; | ||
} | ||
|
||
size_t Type::size() const { | ||
if (_size == UNKNOWN_SIZE) { | ||
return expand().size(); | ||
|
||
} | ||
return _size; | ||
} | ||
|
||
const std::vector<Type>& Type::expand() const { | ||
std::shared_lock<std::shared_timed_mutex> 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; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -992,11 +992,11 @@ Index Function::getLocalIndex(Name name) { | |
Index Function::getVarIndexBase() { return sig.params.size(); } | ||
|
||
Type Function::getLocalType(Index index) { | ||
const std::vector<Type>& params = sig.params.expand(); | ||
if (index < params.size()) { | ||
return params[index]; | ||
auto numParams = sig.params.size(); | ||
if (index < numParams) { | ||
return sig.params.expand()[index]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this also seems like a bad path that we should avoid, as each expand takes a lock? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no way to avoid this right now because fetching an element out of a Type requires accessing the underlying vector. Thread-local caches could help, but that would be future work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. |
||
} else if (isVar(index)) { | ||
return vars[index - params.size()]; | ||
return vars[index - numParams]; | ||
} else { | ||
WASM_UNREACHABLE("invalid local index"); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra unnecessary parens here around
ID_BITS