Re: PSA: mozilla::Result is about to understand move semantics
Just for some additional extra context...
The C++ mozilla::Result implementation is pretty heavily optimized for
common use cases to try to pack its representation into a single machine
word when possible. For instance, if you return a `Result`,
you're really just returning an `nsresult`. If you return a
`Result`, you're really just returning a `T*`. If you return a
`Result`, you're really just returning a `void*`. For the
first two, there isn't any extra overhead compared to not returning a
`Result`. For the third, there are a couple of extra bitwise which might
get optimized out anyway.
It's also fairly easy to add new optimized packing strategies for other
common patterns as they come up.
For more complex types, we currently return a struct. Those calls compile as
the caller allocating space on the stack for the result type and passing in
a pointer (though this varies slightly depending on OS ABI; sometimes larger
types are returned in multiple registers, or large SIMD registers), and the
compiler is generally required to perform copy elision on the return value
if you return a temporary or a local of the same type, i.e., when you write:
Thing foo() {
Thing bar;
bar.mThing = 1;
return bar;
}
`bar` essentially just refers to the pointer that the caller passed in.
The real problem comes when storing large values in, or extracting large
values from, the `Result` itself. In general, if you construct the `Result`
with a named value of any sort, even with `std::move`, it will have to be
copied (unless the compiler can guarantee that there will be no observable
side-effects of omitting the copy, and it's willing to try that hard to
prove that there aren't).
If the `Result` is constructed with a temporary, e.g.,
Result foo() {
return Thing { ... };
}
Then the copy of the temporary `Thing` to the `Result` *may* be omitted, and
is realistically quite likely to be. But if you're doing something like this
in particularly hot code, I'd at least check machine code clang generates
before going very far with it.
-Kris
On Tue, Aug 13, 2019 at 01:37:49PM -0400, Alexis Beingessner wrote:
Just chiming in here with some brief extra context on the performance of
Result (which I really need to do a writeup about so that I can just link
it):
TL;DR performance of Result is usually fine, but can randomly be a huge
problem. However there's also improvements for this on the (distant)
horizon.
Rust and Swift primarily use an error handling strategy based on Result.
For the most part performance is fine, but in some situations you can get
really bad problems. LLVM is very bad at optimizing around Results, and
tends to have copy and branch heavy codegen as a result (which further
hinders other optimizations). This was enough of an issue for the binary
deserializer webrender uses for IPC (bincode) that we just landed a rewrite
to remove the Results (after several attempts by myself to fix the issues
in other ways). [0]
Meanwhile, the Swift compiler team used their expertese in llvm to add new
custom ABIs and calling conventions to handle these performance issues (the
right fix, imo). [1] I need to interview them on these details, to figure
out if we can use their work for Rust. (Since runtime performance is mostly
excellent, it's difficult to motivate working on this and diverting the
limited resources of the Rust team away from the bigger issues like compile
times and async-await.)
Also Meanwhile, the C++ standards committee is apparently[2] investigating
introducing new calling conventions for their new light-weight exceptions
proposal (which is basically adding Result to C++ properly). [3] If that
work goes forward we should be able to take advantage of it for our own C++
code, and possibly also for Rust.
Gonna work on that writeup of this issue now.
[0]: https://bugzilla.mozilla.org/show_bug.cgi?id=1550640
[1]: https://lists.llvm.org/pipermail/llvm-dev/2016-March/096250.html
[2]:
https://botondballo.wordpress.com/2019/07/26/trip-report-c-standards-meeting-in-cologne-july-2019/
[3]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0709r3.pdf
On Tue, Aug 13, 2019 at 1:01 PM Kris Maglione wrote:
On Mon, Aug 12, 2019 at 10:14:19PM -0700, Bryce Seager van Dyk wrote:
>>But either way, that's going to result in a copy when the
>>Result is constructed (unless the compiler is really clever).
>
>Is it the data being moved into the Result which is incurring
>the copy here, or the actual Result that's being returned?
The former.
>I would have thought that the data is moved into the Result
>avoids a copy, then the Result itself would be moved or RVOed
>(either way avoiding a copy).
The move into the result only means that we invoke move rather
than copy constructors when initializing the value stored in the
result. That's more efficient for a lot of things, but still
requires copying data from the old struct to the new one.
The return value Result is guaranteed to
Re: PSA: mozilla::Result is about to understand move semantics
Just chiming in here with some brief extra context on the performance of Result (which I really need to do a writeup about so that I can just link it): TL;DR performance of Result is usually fine, but can randomly be a huge problem. However there's also improvements for this on the (distant) horizon. Rust and Swift primarily use an error handling strategy based on Result. For the most part performance is fine, but in some situations you can get really bad problems. LLVM is very bad at optimizing around Results, and tends to have copy and branch heavy codegen as a result (which further hinders other optimizations). This was enough of an issue for the binary deserializer webrender uses for IPC (bincode) that we just landed a rewrite to remove the Results (after several attempts by myself to fix the issues in other ways). [0] Meanwhile, the Swift compiler team used their expertese in llvm to add new custom ABIs and calling conventions to handle these performance issues (the right fix, imo). [1] I need to interview them on these details, to figure out if we can use their work for Rust. (Since runtime performance is mostly excellent, it's difficult to motivate working on this and diverting the limited resources of the Rust team away from the bigger issues like compile times and async-await.) Also Meanwhile, the C++ standards committee is apparently[2] investigating introducing new calling conventions for their new light-weight exceptions proposal (which is basically adding Result to C++ properly). [3] If that work goes forward we should be able to take advantage of it for our own C++ code, and possibly also for Rust. Gonna work on that writeup of this issue now. [0]: https://bugzilla.mozilla.org/show_bug.cgi?id=1550640 [1]: https://lists.llvm.org/pipermail/llvm-dev/2016-March/096250.html [2]: https://botondballo.wordpress.com/2019/07/26/trip-report-c-standards-meeting-in-cologne-july-2019/ [3]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0709r3.pdf On Tue, Aug 13, 2019 at 1:01 PM Kris Maglione wrote: > On Mon, Aug 12, 2019 at 10:14:19PM -0700, Bryce Seager van Dyk wrote: > >>But either way, that's going to result in a copy when the > >>Result is constructed (unless the compiler is really clever). > > > >Is it the data being moved into the Result which is incurring > >the copy here, or the actual Result that's being returned? > > The former. > > >I would have thought that the data is moved into the Result > >avoids a copy, then the Result itself would be moved or RVOed > >(either way avoiding a copy). > > The move into the result only means that we invoke move rather > than copy constructors when initializing the value stored in the > result. That's more efficient for a lot of things, but still > requires copying data from the old struct to the new one. > > The return value Result is guaranteed to be optimized, though, > so you only wind up with a single copy rather than two. > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: mozilla::Result is about to understand move semantics
On Mon, Aug 12, 2019 at 10:14:19PM -0700, Bryce Seager van Dyk wrote: But either way, that's going to result in a copy when the Result is constructed (unless the compiler is really clever). Is it the data being moved into the Result which is incurring the copy here, or the actual Result that's being returned? The former. I would have thought that the data is moved into the Result avoids a copy, then the Result itself would be moved or RVOed (either way avoiding a copy). The move into the result only means that we invoke move rather than copy constructors when initializing the value stored in the result. That's more efficient for a lot of things, but still requires copying data from the old struct to the new one. The return value Result is guaranteed to be optimized, though, so you only wind up with a single copy rather than two. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Documentation for IPDL needs some edit
Okay, thanks for the confirmation! Will do some needed edits. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Intent to unship: 's mode attribute
Hi, In bug 1573438, I intend to remove support for the mode attribute on the element. This attribute is from MathML 1 and has been deprecated from MathML 2 (released 16 years ago) in favor of the display attribute [1] and we have been sending deprecation warnings since Mozilla 20 (released 6 years ago) [2]. It has never been implemented in WebKit, Igalia does not plan to implement it in Chromium and the MathML Refresh Community Group decided to remove it from MathML Core [3] [1] https://www.w3.org/TR/MathML2/chapter7.html#interf.toplevel [2] https://bugzilla.mozilla.org/show_bug.cgi?id=553917 [3] https://github.com/mathml-refresh/mathml/issues/5 -- Frédéric Wang ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Removing --enable-shared-js [Was: Rust and --enable-shared-js]
Cranelift should be genuinely optional until further notice; to my knownledge, no near-term product work in Firefox or SpiderMonkey depends on Cranelift. Cranelift is present in Nightly but (so far as I can tell) not in Release. It can be disabled in the JS shell by configuring with --disable-cranelift, and I just tested that this works. To the extent there is other Rust code in SpiderMonkey it should not, so far as I know, depend on the presence of Cranelift. It also seems to me that we should be able to use Rust in SpiderMonkey independently of whether Cranelift is there, so if that does not work it ought to be fixed. --lars On Wed, Aug 7, 2019 at 3:53 PM Henri Sivonen wrote: > On Tue, May 28, 2019 at 3:16 AM Mike Hommey wrote: > > > > On Tue, May 21, 2019 at 10:32:20PM -0400, Boris Zbarsky wrote: > > > On 5/21/19 9:55 PM, Mike Hommey wrote: > > > > Considering this has apparently been broken for so long, I guess > nobody > > > > will object to me removing the option for Gecko builds? > > > > > > It's probably fine, yeah... > > > > Now removed on autoland via bug 1554056. > > Thanks. > > It appears that building jsrust_shared is still conditional on > ENABLE_WASM_CRANELIFT. How optional is ENABLE_WASM_CRANELIFT in > practice these days? Is it genuinely optional for Firefox? Is it > genuinely optional for standalone SpiderMonkey? If it is, are we OK > with building without ENABLE_WASM_CRANELIFT having other non-Cranelift > effects on SpiderMonkey performance (e.g. turning off SIMD for some > operations) or on whether a particular string conversion is available > in jsapi.h? > > I'm trying to understand the implication of Cranelift being optional > for other Rust code in SpiderMonkey. I'd like to add Rust-backed > SIMD-accelerated Latin1ness checking and UTF-8 validity checking to > SpiderMonkey and Rust-backed conversion from JSString to UTF-8 in > jsapi.h, and my understanding from All Hands was that adding these > things would be OK, since SpiderMonkey already depends on Rust. > > -- > Henri Sivonen > hsivo...@mozilla.com > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
