Bug 1815177 - Use a custom ReadResult to reduce branching during IPDL deserialization, r=ipc-reviewers,mccr8

Differential Revision: https://phabricator.services.mozilla.com/D169954
This commit is contained in:
Nika Layzell 2023-03-14 19:31:42 +00:00
parent 99ef2e2b47
commit a8e03b0bf5
7 changed files with 215 additions and 57 deletions

View File

@ -1124,9 +1124,9 @@ struct ParamTraits<mozilla::layers::ZoomTarget> {
MOZ_ASSERT(rv, "Serialize ##type_## failed"); \
WriteParam(aWriter, std::move(v)); \
} \
static mozilla::Maybe<paramType> Read(MessageReader* aReader) { \
static ReadResult<paramType> Read(MessageReader* aReader) { \
mozilla::ipc::ByteBuf in; \
mozilla::Maybe<paramType> result; \
ReadResult<paramType> result; \
if (!ReadParam(aReader, &in) || !in.mData) { \
return result; \
} \
@ -1135,7 +1135,7 @@ struct ParamTraits<mozilla::layers::ZoomTarget> {
if (!Servo_##type_##_Deserialize(&in, value.addr())) { \
return result; \
} \
result.emplace(std::move(*value.addr())); \
result = std::move(*value.addr()); \
value.addr()->~paramType(); \
return result; \
} \

View File

@ -208,6 +208,156 @@ class MOZ_STACK_CLASS MessageReader final {
mozilla::ipc::IProtocol* actor_;
};
namespace detail {
// Helper for checking `T::kHasDeprecatedReadParamPrivateConstructor` using a
// fallback when the member isn't defined.
template <typename T>
inline constexpr auto HasDeprecatedReadParamPrivateConstructor(int)
-> decltype(T::kHasDeprecatedReadParamPrivateConstructor) {
return T::kHasDeprecatedReadParamPrivateConstructor;
}
template <typename T>
inline constexpr bool HasDeprecatedReadParamPrivateConstructor(...) {
return false;
}
} // namespace detail
/**
* Result type returned from some `ParamTraits<T>::Read` implementations, and
* from `IPC::ReadParam<T>(MessageReader*)`. Either contains the value or
* indicates a failure to deserialize.
*
* This type can be thought of as a variant on `Maybe<T>`, except that it
* unconditionally constructs the underlying value if it is default
* constructible. This helps keep code size down, especially when calling
* outparameter-based ReadParam implementations (bug 1815177).
*/
template <typename T,
bool = std::is_default_constructible_v<T> ||
detail::HasDeprecatedReadParamPrivateConstructor<T>(0)>
class ReadResult {
public:
ReadResult() = default;
template <typename U, std::enable_if_t<std::is_convertible_v<U, T>, int> = 0>
MOZ_IMPLICIT ReadResult(U&& aData)
: mIsOk(true), mData(std::forward<U>(aData)) {}
template <typename... Args>
explicit ReadResult(std::in_place_t, Args&&... aArgs)
: mIsOk(true), mData(std::forward<Args>(aArgs)...) {}
ReadResult(const ReadResult&) = default;
ReadResult(ReadResult&&) = default;
template <typename U, std::enable_if_t<std::is_convertible_v<U, T>, int> = 0>
MOZ_IMPLICIT ReadResult& operator=(U&& aData) {
mIsOk = true;
mData = std::forward<U>(aData);
return *this;
}
ReadResult& operator=(const ReadResult&) = default;
ReadResult& operator=(ReadResult&&) noexcept = default;
// Check if the ReadResult contains a valid value.
explicit operator bool() const { return isOk(); }
bool isOk() const { return mIsOk; }
// Get the data from this ReadResult.
T& get() {
MOZ_ASSERT(mIsOk);
return mData;
}
const T& get() const {
MOZ_ASSERT(mIsOk);
return mData;
}
T& operator*() { return get(); }
const T& operator*() const { return get(); }
T* operator->() { return &get(); }
const T* operator->() const { return &get(); }
// Try to extract a `Maybe<T>` from this ReadResult.
mozilla::Maybe<T> TakeMaybe() {
if (mIsOk) {
mIsOk = false;
return mozilla::Some(std::move(mData));
}
return mozilla::Nothing();
}
// Get the underlying data from this ReadResult, even if not OK.
//
// This is only available for types which are default constructible, and is
// used to optimize old-style `ReadParam` calls.
T& GetStorage() { return mData; }
// Compliment to `GetStorage` used to set the ReadResult into an OK state
// without constructing the underlying value.
void SetOk(bool aIsOk) { mIsOk = aIsOk; }
private:
bool mIsOk = false;
T mData{};
};
template <typename T>
class ReadResult<T, false> {
public:
ReadResult() = default;
template <typename U, std::enable_if_t<std::is_convertible_v<U, T>, int> = 0>
MOZ_IMPLICIT ReadResult(U&& aData)
: mData(std::in_place, std::forward<U>(aData)) {}
template <typename... Args>
explicit ReadResult(std::in_place_t, Args&&... aArgs)
: mData(std::in_place, std::forward<Args>(aArgs)...) {}
ReadResult(const ReadResult&) = default;
ReadResult(ReadResult&&) = default;
template <typename U, std::enable_if_t<std::is_convertible_v<U, T>, int> = 0>
MOZ_IMPLICIT ReadResult& operator=(U&& aData) {
mData.reset();
mData.emplace(std::forward<U>(aData));
return *this;
}
ReadResult& operator=(const ReadResult&) = default;
ReadResult& operator=(ReadResult&&) noexcept = default;
// Check if the ReadResult contains a valid value.
explicit operator bool() const { return isOk(); }
bool isOk() const { return mData.isSome(); }
// Get the data from this ReadResult.
T& get() { return mData.ref(); }
const T& get() const { return mData.ref(); }
T& operator*() { return get(); }
const T& operator*() const { return get(); }
T* operator->() { return &get(); }
const T* operator->() const { return &get(); }
// Try to extract a `Maybe<T>` from this ReadResult.
mozilla::Maybe<T> TakeMaybe() { return std::move(mData); }
// These methods are only available if the type is default constructible.
T& GetStorage() = delete;
void SetOk(bool aIsOk) = delete;
private:
mozilla::Maybe<T> mData;
};
//-----------------------------------------------------------------------------
// An iterator class for reading the fields contained within a Message.
@ -313,33 +463,24 @@ template <typename P>
inline bool WARN_UNUSED_RESULT ReadParam(MessageReader* reader, P* p) {
if constexpr (!detail::ParamTraitsReadUsesOutParam<P>()) {
auto maybe = ParamTraits<P>::Read(reader);
if (maybe.isNothing()) {
return false;
if (maybe) {
*p = std::move(*maybe);
return true;
}
*p = maybe.extract();
return true;
return false;
} else {
return ParamTraits<P>::Read(reader, p);
}
}
template <typename P>
inline mozilla::Maybe<P> WARN_UNUSED_RESULT ReadParam(MessageReader* reader) {
inline ReadResult<P> WARN_UNUSED_RESULT ReadParam(MessageReader* reader) {
if constexpr (!detail::ParamTraitsReadUsesOutParam<P>()) {
return ParamTraits<P>::Read(reader);
} else if constexpr (std::is_default_constructible_v<P>) {
mozilla::Maybe<P> p{std::in_place};
if (!ParamTraits<P>::Read(reader, p.ptr())) {
p.reset();
}
return p;
} else {
static_assert(P::kHasDeprecatedReadParamPrivateConstructor);
P p{};
if (!ParamTraits<P>::Read(reader, &p)) {
return mozilla::Nothing();
}
return mozilla::Some(std::move(p));
ReadResult<P> p;
p.SetOk(ParamTraits<P>::Read(reader, &p.GetStorage()));
return p;
}
}
@ -507,7 +648,7 @@ bool ReadSequenceParamImpl(MessageReader* reader, mozilla::Maybe<I>&& data,
if (!elt) {
return false;
}
*data.ref() = elt.extract();
*data.ref() = std::move(*elt);
++data.ref();
}
return true;
@ -920,16 +1061,16 @@ struct ParamTraitsMozilla<mozilla::NotNull<T>> {
ParamTraits<T>::Write(writer, p.get());
}
static mozilla::Maybe<mozilla::NotNull<T>> Read(MessageReader* reader) {
static ReadResult<mozilla::NotNull<T>> Read(MessageReader* reader) {
auto ptr = ReadParam<T>(reader);
if (!ptr) {
return mozilla::Nothing();
return {};
}
if (!*ptr) {
reader->FatalError("unexpected null value");
return mozilla::Nothing();
return {};
}
return mozilla::Some(mozilla::WrapNotNull(std::move(*ptr)));
return mozilla::WrapNotNull(std::move(*ptr));
}
};

View File

@ -22,14 +22,17 @@ namespace IPC {
class Message;
class MessageReader;
class MessageWriter;
template <typename P>
inline mozilla::Maybe<P> ReadParam(MessageReader*);
template <typename T, bool>
class ReadResult;
} // namespace IPC
// TODO(bug 1812271): Remove users of this macro.
#define ALLOW_DEPRECATED_READPARAM \
public: \
enum { kHasDeprecatedReadParamPrivateConstructor = true }; \
template <typename P> \
friend mozilla::Maybe<P> IPC::ReadParam(IPC::MessageReader*);
template <typename, bool> \
friend class IPC::ReadResult; \
\
private:
#endif

View File

@ -458,7 +458,7 @@ struct ParamTraits<mozilla::Maybe<T>> {
return false;
}
if (isSome) {
mozilla::Maybe<T> tmp = ReadParam<T>(reader);
mozilla::Maybe<T> tmp = ReadParam<T>(reader).TakeMaybe();
if (!tmp) {
return false;
}

View File

@ -161,16 +161,24 @@ struct ParamTraits<mozilla::ipc::SideVariant<ParentSide, ChildSide>> {
}
}
static mozilla::Maybe<paramType> Read(IPC::MessageReader* aReader) {
static ReadResult<paramType> Read(IPC::MessageReader* aReader) {
if (!aReader->GetActor()) {
aReader->FatalError("actor required to deserialize this type");
return mozilla::Nothing();
return {};
}
if (aReader->GetActor()->GetSide() == mozilla::ipc::ParentSide) {
return ReadParam<ParentSide>(aReader);
auto parentSide = ReadParam<ParentSide>(aReader);
if (!parentSide) {
return {};
}
return std::move(*parentSide);
}
return ReadParam<ChildSide>(aReader);
auto childSide = ReadParam<ChildSide>(aReader);
if (!childSide) {
return {};
}
return std::move(*childSide);
}
};

View File

@ -855,16 +855,6 @@ class ExprMove(ExprCall):
ExprCall.__init__(self, ExprVar("std::move"), args=[arg])
class ExprNothing(ExprCall):
def __init__(self):
ExprCall.__init__(self, ExprVar("mozilla::Nothing"))
class ExprSome(ExprCall):
def __init__(self, arg):
ExprCall.__init__(self, ExprVar("mozilla::Some"), args=[arg])
class ExprNew(Node):
# XXX taking some poetic license ...
def __init__(self, ctype, args=[], newargs=None):

View File

@ -317,6 +317,16 @@ def _cxxMaybeType(basetype, const=False, ref=False):
)
def _cxxReadResultType(basetype, const=False, ref=False):
return Type(
"IPC::ReadResult",
T=basetype,
const=const,
ref=ref,
hasimplicitcopyctor=basetype.hasimplicitcopyctor,
)
def _cxxNotNullType(basetype, const=False, ref=False):
return Type(
"mozilla::NotNull",
@ -462,6 +472,10 @@ def errfnUnreachable(msg):
return [_logicError(msg)]
def readResultError():
return ExprCode("{}")
class _DestroyReason:
@staticmethod
def Type():
@ -2016,14 +2030,14 @@ class _ParamTraits:
ifbad = StmtIf(ExprNot(readbytes))
errmsg = "Error bulk reading fields from %s" % first.ipdltype.name()
ifbad.addifstmts(
[cls.fatalError(cls.readervar, errmsg), StmtReturn(ExprNothing())]
[cls.fatalError(cls.readervar, errmsg), StmtReturn(readResultError())]
)
block.addstmt(ifbad)
block.addstmts(
cls.readSentinel(
cls.readervar,
cls.bulkSentinelKey(fields),
errfnSentinel(ExprNothing())(errmsg),
errfnSentinel(readResultError())(errmsg),
)
)
@ -2074,7 +2088,7 @@ class _ParamTraits:
@classmethod
def _checkedRead(cls, ipdltype, cxxtype, var, sentinelKey, what):
def errfn(msg):
return [cls.fatalError(cls.readervar, msg), StmtReturn(ExprNothing())]
return [cls.fatalError(cls.readervar, msg), StmtReturn(readResultError())]
return cls.checkedRead(
ipdltype,
@ -2084,7 +2098,7 @@ class _ParamTraits:
errfn=errfn,
paramtype=what,
sentinelKey=sentinelKey,
errfnSentinel=errfnSentinel(ExprNothing()),
errfnSentinel=errfnSentinel(readResultError()),
)
@classmethod
@ -2119,15 +2133,14 @@ class _ParamTraits:
writemthd.addstmts(write)
pt.addstmt(writemthd)
# static Maybe<T> Read(MessageReader*);
outtype = Type("paramType", ptr=True)
# static ReadResult<T> Read(MessageReader*);
readmthd = MethodDefn(
MethodDecl(
"Read",
params=[
Decl(Type("IPC::MessageReader", ptr=True), cls.readervar.name),
],
ret=Type("mozilla::Maybe<paramType>"),
ret=Type("IPC::ReadResult<paramType>"),
methodspec=MethodSpec.STATIC,
)
)
@ -2185,9 +2198,12 @@ class _ParamTraits:
MOZ_RELEASE_ASSERT(
${readervar}->GetActor(),
"Cannot deserialize managed actors without an actor");
return ${readervar}->GetActor()
->ReadActor(${readervar}, true, ${actortype}, ${protocolid})
.map([](mozilla::ipc::IProtocol* actor) { return static_cast<${cxxtype}>(actor); });
mozilla::Maybe<mozilla::ipc::IProtocol*> actor = ${readervar}->GetActor()
->ReadActor(${readervar}, true, ${actortype}, ${protocolid});
if (actor.isSome()) {
return static_cast<${cxxtype}>(actor.ref());
}
return {};
""",
readervar=cls.readervar,
actortype=ExprLiteral.String(actortype.name()),
@ -2249,7 +2265,7 @@ class _ParamTraits:
resultvar = ExprVar("result__")
read.append(
StmtDecl(
Decl(_cxxMaybeType(Type("paramType")), resultvar.name),
Decl(_cxxReadResultType(Type("paramType")), resultvar.name),
initargs=[ExprVar("std::in_place")] + ctorargs,
)
)
@ -2333,7 +2349,7 @@ class _ParamTraits:
origenum,
"variant " + origenum + " of union " + uniontype.name(),
),
StmtReturn(ExprSome(ExprMove(tmpvar))),
StmtReturn(ExprMove(tmpvar)),
]
)
readswitch.addcase(caselabel, readcase)
@ -2357,7 +2373,7 @@ class _ParamTraits:
cls.fatalError(
cls.readervar, "unknown variant of union " + uniontype.name()
),
StmtReturn(ExprNothing()),
StmtReturn(readResultError()),
]
),
)