Closed Bug 1626570 Opened 7 months ago Closed 6 months ago

Cannot declare nsTArray<IncompleteType> anymore / nsTArray should not be copyable in most contexts

Categories

(Core :: XPCOM, defect)

defect

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox-esr68 --- unaffected
firefox75 --- unaffected
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- fixed

People

(Reporter: emilio, Assigned: sg)

References

(Regression)

Details

(Keywords: regression)

Attachments

(87 files, 4 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
struct MyStruct;

struct Bar {
  Bar();
  ~Bar();
  nsTArray<MyStruct> mArray;
};

Should probably work.

Flags: needinfo?(sgiesecke)

If you really only have a forward declaration of MyStruct, you need to declare whether MyStruct is copy-constructible by either MOZ_DECLARE_COPY_CONSTRUCTIBLE or MOZ_DECLARE_NON_COPY_CONSTRUCTIBLE.

Flags: needinfo?(sgiesecke)

Why? This works with std::vector and so on.

Can we move the template magic from CopyEnabler to the copy-constructor so that you only need to evaluate IsCopyConstructible if you try to call it?

Though I guess std::vector is copy-constructible even if T isn't... I don't understand the reasoning for https://phabricator.services.mozilla.com/D66244, can you elaborate? This will cause people to just add a bunch of otherwise-useless includes...

Flags: needinfo?(sgiesecke)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Why? This works with std::vector and so on.

Yes, it works with std::vector in libstdc++ and libc++. Not sure if this is required by the standard (maybe yes), and what exactly "and so on" includes, but probably it works with a number of other containers as well. But does that imply there shouldn't be any containers that rely on a complete definition of its argument types? (which definitely isn't true for templates in general other than containers)

Can we move the template magic from CopyEnabler to the copy-constructor so that you only need to evaluate IsCopyConstructible if you try to call it?

I am not completely sure what you suggest here. Can you make this more specific? Adding any user-provided copy constructor to nsTArray_Impl would defeat the purpose of the CopyEnabler.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

Though I guess std::vector is copy-constructible even if T isn't...

I tried it and you're right insofar static_assert(std::is_copy_constructible_v<std::vector<std::unique_ptr<int>>>); holds, but of course the copy constructor cannot be instantiated either.

I don't understand the reasoning for https://phabricator.services.mozilla.com/D66244, can you elaborate?

I don't remember exactly, but it was somehow related to gcc choosing the copy constructor or assignment operator, which couldn't be instantiated. Try to revert https://phabricator.services.mozilla.com/D66244 and https://phabricator.services.mozilla.com/D66247 to see what happens when compiling with gcc then. If you don't want to do that, I can check it again. I am not completely sure why clang doesn't bail out as well.

I am not sure if this is only an outcome of the existing of FallibleTArray (which ought to be removed eventually) and/or AutoTArray deriving from nsTArray and/or many of our types missing to specify noexcept on their move operations. Just speculating.

In general, I think it's not good if a type claims std::is_copy_constructible but actually it isn't. It seems to defeat the purpose of these type traits and renders them useless.

This will cause people to just add a bunch of otherwise-useless includes...

I am not sure what you mean by "useless" here. The Google coding style, which we adopt, discourages the use of forward declarations: https://google.github.io/styleguide/cppguide.html#Forward_Declarations

Flags: needinfo?(sgiesecke)

(In reply to Simon Giesecke [:sg] [he/him] from comment #4)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Why? This works with std::vector and so on.

Yes, it works with std::vector in libstdc++ and libc++. Not sure if this is required by the standard (maybe yes), and what exactly "and so on" includes, but probably it works with a number of other containers as well. But does that imply there shouldn't be any containers that rely on a complete definition of its argument types? (which definitely isn't true for templates in general other than containers)

Can we move the template magic from CopyEnabler to the copy-constructor so that you only need to evaluate IsCopyConstructible if you try to call it?

I am not completely sure what you suggest here. Can you make this more specific? Adding any user-provided copy constructor to nsTArray_Impl would defeat the purpose of the CopyEnabler.

I meant something like:

template <typename = typename std::enable_if_t<std::is_copy_constructible_v<E>>>
nsTArray(const nsTArray&);

or something. But I couldn't make something like that work.

This will cause people to just add a bunch of otherwise-useless includes...

I am not sure what you mean by "useless" here. The Google coding style, which we adopt, discourages the use of forward declarations: https://google.github.io/styleguide/cppguide.html#Forward_Declarations

I don't read "Try to avoid forward declarations of entities defined in another project" as "discourages the use of forward declarations"... Forward declarations are one of the best ways to avoid blowing up compile times.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

(In reply to Simon Giesecke [:sg] [he/him] from comment #4)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Why? This works with std::vector and so on.

Yes, it works with std::vector in libstdc++ and libc++. Not sure if this is required by the standard (maybe yes), and what exactly "and so on" includes, but probably it works with a number of other containers as well. But does that imply there shouldn't be any containers that rely on a complete definition of its argument types? (which definitely isn't true for templates in general other than containers)

Can we move the template magic from CopyEnabler to the copy-constructor so that you only need to evaluate IsCopyConstructible if you try to call it?

I am not completely sure what you suggest here. Can you make this more specific? Adding any user-provided copy constructor to nsTArray_Impl would defeat the purpose of the CopyEnabler.

I meant something like:

template <typename = typename std::enable_if_t<std::is_copy_constructible_v<E>>>
nsTArray(const nsTArray&);

or something. But I couldn't make something like that work.

Ok, then it's what I thought. Yes, that doesn't work. You can't provide a copy constructor conditionally relying on SFINAE, this is the very reason the CopyEnabler exists. What could be done differently in theory were not derive from a class template depending on the type trait but having this on a data member instead, but I am not sure if this would change anything about requiring a complete definition of the type.

This will cause people to just add a bunch of otherwise-useless includes...

I am not sure what you mean by "useless" here. The Google coding style, which we adopt, discourages the use of forward declarations: https://google.github.io/styleguide/cppguide.html#Forward_Declarations

I don't read "Try to avoid forward declarations of entities defined in another project" as "discourages the use of forward declarations"... Forward declarations are one of the best ways to avoid blowing up compile times.

Well, the wording is not entirely clear. The first sentence under the heading has no restriction to "another project" (whatever that means) and the two bullet points after the one you cited don't have it either. It may have a significant influence on compile-times in the case it is a widely included header and its users don't require the definitions of the involved types anyway. Personally, I would still try to address this in some way other than with forward declarations, but this is somewhat out of scope here.

Getting back to the scope of the bug: is using the macros I mentioned or providing a definition of the type acceptable here?

Getting back to the scope of the bug: is using the macros I mentioned or providing a definition of the type acceptable here?

Well, not sure... I sure could make it work like that, or I can add the #include. If I use the macro then there's two pieces of code that I need to keep track of. In this case it was an ipdl-generated type. If the code generation changes, then the macro usage can get out of sync.

Plus in this case I don't even care if the array is copy-constructible or not, I just have a single array where I emplace elements and move away at some point.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Getting back to the scope of the bug: is using the macros I mentioned or providing a definition of the type acceptable here?

Well, not sure... I sure could make it work like that, or I can add the #include. If I use the macro then there's two pieces of code that I need to keep track of. In this case it was an ipdl-generated type. If the code generation changes, then the macro usage can get out of sync.

Can you share the patch with me?

Plus in this case I don't even care if the array is copy-constructible or not, I just have a single array where I emplace elements and move away at some point.

You could just use MOZ_DECLARE_NON_COPY_CONSTRUCTIBLE then and get a non-copyable nsTArray even if the element type is copyable.
(And if you wrongfully used MOZ_DECLARE_COPY_CONSTRUCTIBLE, no bad things happen. I am not completely sure, it either will fail to compile at all or you get back to the state before D66244, with the nsTArray claiming is_copy_constructible, but copy operations that cannot be instantiated).

Another option might be to use mozilla/Vector.h?

After doing some more work on nsTArray, I think that nsTArray should be made uncopyable in general. Copies may be expensive, so doing this accidentally/implicitly doesn't seem a good idea. We can add CopyableTArray for the rare cases where this is really required, and no named member functions can be used explicitly instead. See also Bug 1628692, which is about FallibleTArray, where this is even more problematic.

For the regular nsTArray, this will allow the use with incomplete types again. Also, MOZ_DECLARE_NON_COPY_CONSTRUCTIBLE and MOZ_DECLARE_COPY_CONSTRUCTIBLE can be removed again.

Assignee: nobody → sgiesecke
Status: NEW → ASSIGNED
Attachment #9143030 - Attachment description: Bug 1626570 - Improve handling of copying arrays in layout/generic/. r=emilio → Bug 1626570 - Improve handling of copying arrays in layout/base/. r=emilio
Blocks: 1633675
Keywords: leave-open
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8d629c316f9
Add CopyableTArray. r=xpcom-reviewers,nika
https://hg.mozilla.org/integration/autoland/rev/0ca000a364b4
Add ParamTraits for CopyableTArray. r=nika
https://hg.mozilla.org/integration/autoland/rev/078ec53cf0f5
Improve handling of copying arrays in tools/profiler/gecko. r=mstange
https://hg.mozilla.org/integration/autoland/rev/646f434f456d
Improve handling of copying arrays in accessible/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/2505fccc09c9
Improve handling of copying arrays in extensions/spellcheck. r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fdef6c89e60f
Improve handling of copying arrays in dom/ipc/. r=nika
https://hg.mozilla.org/integration/autoland/rev/ef52aacd0d0c
Improve handling of copying arrays in widget/. r=jhorak
https://hg.mozilla.org/integration/autoland/rev/edb8086fa7b9
Improve handling of copying arrays in uriloader/. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/a883227e3bf3
Improve handling of copying arrays in toolkit/components/places/. r=mak
https://hg.mozilla.org/integration/autoland/rev/0648d1512c8e
Improve handling of copying arrays in toolkit/components/perfmonitoring/. r=tarek
https://hg.mozilla.org/integration/autoland/rev/0dabfb14c68a
Improve handling of copying arrays in toolkit/components/extensions/. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/e1e24ef3b06c
Improve handling of copying arrays in toolkit/components/telemetry/. r=chutten
https://hg.mozilla.org/integration/autoland/rev/79aa34a3c0e2
Improve handling of copying arrays in toolkit/components/url-classifier/. r=gcp
https://hg.mozilla.org/integration/autoland/rev/ecd8d7595c98
Improve handling of copying arrays in toolkit/components/sessionstore/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/1d6e1743691e
Improve handling of copying arrays in toolkit/components/antitracking/. r=baku
https://hg.mozilla.org/integration/autoland/rev/b0395e5dc5e6
Improve handling of copying arrays in netwerk/url-classifier/. r=dimi
Attachment #9145437 - Attachment is obsolete: true
Blocks: 1635256
Summary: Cannot declare nsTArray<IncompleteType> anymore. → Cannot declare nsTArray<IncompleteType> anymore / nsTArray should not be copyable in most contexts
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6c06d397a5d2
Improve handling of copying arrays in security/manager/ssl/. r=keeler
https://hg.mozilla.org/integration/autoland/rev/a7d4e3a59ee3
Improve handling of copying arrays in netwerk/dns/. r=valentin,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/52d0911d4ad3
Improve handling of copying arrays in netwerk/base/. r=valentin,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/68b4c2418d83
Improve handling of copying arrays in media/webrtc/signaling/src/. r=bwc
https://hg.mozilla.org/integration/autoland/rev/793f978248b3
Improve handling of copying arrays in js/xpconnect/. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/e85450d69351
Improve handling of copying arrays in intl/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/4c69a4c013b3
Improve handling of copying arrays in layout/style/ and servo/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/c339fd44c9f8
Improve handling of copying arrays in layout/base/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/5247e1ddd5d6
Improve handling of copying arrays in layout/mathml/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/a3f17d392234
Improve handling of copying arrays in layout/inspector/. r=emilio

D73642 must probably land together this those patches. I am checking this again, and will then re-land.

Flags: needinfo?(sgiesecke)
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/00793c67119e
Improve handling of copying arrays in security/manager/ssl/. r=keeler
https://hg.mozilla.org/integration/autoland/rev/b698a70baad9
Improve handling of copying arrays in netwerk/dns/. r=valentin,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/6a15c418a163
Improve handling of copying arrays in netwerk/base/. r=valentin,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/ebc95a347a4d
Improve handling of copying arrays in media/webrtc/signaling/src/. r=bwc
https://hg.mozilla.org/integration/autoland/rev/a4394254942f
Improve handling of copying arrays in js/xpconnect/. r=mccr8
https://hg.mozilla.org/integration/autoland/rev/b82c21f49cf0
Improve handling of copying arrays in intl/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/cb92328d2f98
Improve handling of copying arrays in dom/animation/. r=hiro
https://hg.mozilla.org/integration/autoland/rev/ea2ebdfc8420
Improve handling of copying arrays in layout/style/ and servo/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/fcb1c15017d7
Improve handling of copying arrays in layout/base/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/209f82a9305c
Improve handling of copying arrays in layout/mathml/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/0a74c3e1493e
Improve handling of copying arrays in layout/inspector/. r=emilio
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bc91cd947dce
Add CopyableAutoTArray. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/a2a27f276532
Improve handling of copying arrays in gfx/thebes. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/81b877a1173d
Improve handling of copying arrays in layout/painting/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/845efef1d2f6
Improve handling of copying arrays in layout/generic/. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/525842a74603
Improve handling of copying arrays in xpcom/. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/51b22311b4d3
Improve handling of copying arrays in dom/base/. r=emilio
https://hg.mozilla.org/integration/autoland/rev/1af19ba39456
Improve handling of copying arrays in dom/html/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/0cb3cee0d145
Improve handling of copying arrays in dom/media/. r=bryce
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/10abd1ef75fb
Improve handling of copying arrays in dom/quota/. r=dom-workers-and-storage-reviewers,janv
https://hg.mozilla.org/integration/autoland/rev/7a510d21948f
Improve handling of copying arrays in dom/smil/. r=birtles
https://hg.mozilla.org/integration/autoland/rev/ae0a12a3935f
Improve handling of copying arrays in gfx/layers/. r=botond
https://hg.mozilla.org/integration/autoland/rev/704cedb103b8
Improve handling of copying arrays in gfx/. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/078ce9d263f9
Improve handling of copying arrays in hal/. r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/b045baf40b3b
Improve handling of copying arrays in netwerk/sctp/datachannel/. r=drno
https://hg.mozilla.org/integration/autoland/rev/2136c5964756
Improve handling of copying arrays in MozPromise. r=froydnj
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/68cc4148bda2
Improve handling of copying arrays in dom/clients/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/44523b655777
Improve handling of copying arrays in dom/events/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/442990d2263c
Improve handling of copying arrays in dom/grid/. r=mats
https://hg.mozilla.org/integration/autoland/rev/a83bdead5cf4
Improve handling of copying arrays in dom/indexedDB/. r=dom-workers-and-storage-reviewers,janv
https://hg.mozilla.org/integration/autoland/rev/beb39c53935c
Improve handling of copying arrays in dom/l10n/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/2440385dd6ed
Improve handling of copying arrays in dom/fetch/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/01d9c38b8b92
Improve handling of copying arrays in dom/payments/. r=baku
https://hg.mozilla.org/integration/autoland/rev/2b8cca4379a3
Improve handling of copying arrays in dom/performance/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/400ef1fdd137
Improve handling of copying arrays in dom/canvas/. r=lsalzman
https://hg.mozilla.org/integration/autoland/rev/515a0edacecf
Improve handling of copying arrays in dom/filesystem/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/3b4806b00aec
Improve handling of copying arrays in dom/push/. r=lina
https://hg.mozilla.org/integration/autoland/rev/4ff37b98727e
Improve handling of copying arrays in dom/reporting/. r=smaug

https://hg.mozilla.org/mozilla-central/rev/00793c67119e
https://hg.mozilla.org/mozilla-central/rev/b698a70baad9
https://hg.mozilla.org/mozilla-central/rev/6a15c418a163
https://hg.mozilla.org/mozilla-central/rev/ebc95a347a4d
https://hg.mozilla.org/mozilla-central/rev/a4394254942f
https://hg.mozilla.org/mozilla-central/rev/b82c21f49cf0
https://hg.mozilla.org/mozilla-central/rev/cb92328d2f98
https://hg.mozilla.org/mozilla-central/rev/ea2ebdfc8420
https://hg.mozilla.org/mozilla-central/rev/fcb1c15017d7
https://hg.mozilla.org/mozilla-central/rev/209f82a9305c
https://hg.mozilla.org/mozilla-central/rev/0a74c3e1493e
https://hg.mozilla.org/mozilla-central/rev/bc91cd947dce
https://hg.mozilla.org/mozilla-central/rev/a2a27f276532
https://hg.mozilla.org/mozilla-central/rev/81b877a1173d
https://hg.mozilla.org/mozilla-central/rev/845efef1d2f6
https://hg.mozilla.org/mozilla-central/rev/525842a74603
https://hg.mozilla.org/mozilla-central/rev/51b22311b4d3
https://hg.mozilla.org/mozilla-central/rev/1af19ba39456
https://hg.mozilla.org/mozilla-central/rev/0cb3cee0d145
https://hg.mozilla.org/mozilla-central/rev/10abd1ef75fb
https://hg.mozilla.org/mozilla-central/rev/7a510d21948f
https://hg.mozilla.org/mozilla-central/rev/ae0a12a3935f
https://hg.mozilla.org/mozilla-central/rev/704cedb103b8
https://hg.mozilla.org/mozilla-central/rev/078ce9d263f9
https://hg.mozilla.org/mozilla-central/rev/b045baf40b3b
https://hg.mozilla.org/mozilla-central/rev/2136c5964756
https://hg.mozilla.org/mozilla-central/rev/68cc4148bda2
https://hg.mozilla.org/mozilla-central/rev/44523b655777
https://hg.mozilla.org/mozilla-central/rev/442990d2263c
https://hg.mozilla.org/mozilla-central/rev/a83bdead5cf4
https://hg.mozilla.org/mozilla-central/rev/beb39c53935c
https://hg.mozilla.org/mozilla-central/rev/2440385dd6ed
https://hg.mozilla.org/mozilla-central/rev/01d9c38b8b92
https://hg.mozilla.org/mozilla-central/rev/2b8cca4379a3
https://hg.mozilla.org/mozilla-central/rev/400ef1fdd137
https://hg.mozilla.org/mozilla-central/rev/515a0edacecf
https://hg.mozilla.org/mozilla-central/rev/3b4806b00aec
https://hg.mozilla.org/mozilla-central/rev/4ff37b98727e

Blocks: 1635689
Attachment #9145425 - Attachment is obsolete: true
Blocks: 1635713
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/21d9662b7139
Improve handling of copying arrays in editor/. r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8e28d10177c0
Improve handling of copying arrays in dom/plugins/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/e8360d8d9140
Improve handling of copying arrays in dom/security/. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/90087b975479
Improve handling of copying arrays in dom/webauthn. r=jcj
https://hg.mozilla.org/integration/autoland/rev/340387460cea
Improve handling of copying arrays in dom/webgpu/. r=kvark
https://hg.mozilla.org/integration/autoland/rev/1cf33b46f79c
Improve handling of copying arrays in dom/vr/. r=kip
https://hg.mozilla.org/integration/autoland/rev/0c3511b04a08
Improve handling of copying arrays in dom/console/. r=baku
https://hg.mozilla.org/integration/autoland/rev/f632c3de9167
Improve handling of copying arrays in dom/network/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/047eb92da0b8
Improve handling of copying arrays in dom/midi/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/f876865a0809
Improve handling of copying arrays in dom/notification/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/bdf50b0f550e
Improve handling of copying arrays in dom/presentation/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/242b4a61389a
Improve handling of copying arrays in gfx/vr/. r=kip
https://hg.mozilla.org/integration/autoland/rev/b2957781f934
Improve handling of copying arrays in image/. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/710da9ba5df9
Improve handling of copying arrays in netwerk/protocol/. r=valentin,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/db5a127f11e4
Improve handling of copying arrays in netwerk/. r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/aa89500e0794
Improve handling of copying arrays in ipc/. r=nika
https://hg.mozilla.org/integration/autoland/rev/8b451c877b10
Improve handling of copying arrays in dom/ipc/. r=nika
https://hg.mozilla.org/integration/autoland/rev/49bbb28670a3
Improve handling of copying arrays in mobile/. r=geckoview-reviewers,esawin
https://hg.mozilla.org/integration/autoland/rev/69f7e3e32072
Improve handling of copying arrays in storage/. r=mak
https://hg.mozilla.org/integration/autoland/rev/186eac923253
Improve handling of copying arrays in dom/power/. r=smaug
Blocks: 1636047
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2b205fe4f496
Improve handling of copying arrays in dom/html/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/6b2268b85ba9
Improve handling of copying arrays in dom/messagechannel/. r=smaug
https://hg.mozilla.org/integration/autoland/rev/6ff464bd2190
Use CopyableTArray in ipdlc as member type for now. r=nika
https://hg.mozilla.org/integration/autoland/rev/b60235fc5d62
Improve handling of copying arrays in widget/. r=jhorak
https://hg.mozilla.org/integration/autoland/rev/c9cecffc0718
Improve handling of copying arrays in dom/cache/. r=dom-workers-and-storage-reviewers,ttung
Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5d64fb7cb091
FIX-toolchain-bustage. CLOSED TREE
Severity: normal → S4
Keywords: leave-open
Pushed by sgiesecke@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9b53d010f336
Improve handling of copying arrays in dom/gamepad/. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/7b02e379d198
Improve handling of copying arrays in dom/xslt/. r=peterv
https://hg.mozilla.org/integration/autoland/rev/66d730089275
Improve handling of copying arrays in dom/serviceworkers/. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/d80ba93e17be
Improve handling of copying arrays in dom/workers/. r=dom-workers-and-storage-reviewers,ytausky
https://hg.mozilla.org/integration/autoland/rev/8e8a16e4e40d
Improve handling of copying arrays in toolkit/mozapps/. r=aswan
https://hg.mozilla.org/integration/autoland/rev/93b096e2d3da
Improve handling of copying arrays in dom/bindings/. r=peterv
https://hg.mozilla.org/integration/autoland/rev/b65169d47a91
Make nsTArray non-copyable. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/7b7db00669fe
Remove obsolete copy enabling machinery. r=froydnj
Attachment #9142760 - Attachment is obsolete: true
Attachment #9142989 - Attachment is obsolete: true

You probably refer to this step specifically? https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=autoland&newProject=autoland&originalRevision=d2bb25cf6ed55b09585f40f102d514f679469c33&newRevision=2136c5964756e8a9fc04d755986e907588588385&originalSignature=1921248&newSignature=1921248&framework=10#table-row-1197107037

If anything, improvements were expected where arrays are now moved which were copied before. What is conceivable is that copy elision might have been in effect before? The results are very mixed, some of the sub-benchmarks have improved, others have worsened. Can you point me to instructions to run these locally, so I can run them under a profiler and get a better understanding of what's happening?

Flags: needinfo?(sgiesecke) → needinfo?(dmajor)

The results are very mixed, some of the sub-benchmarks have improved, others have worsened.

The subtest viewer is not reliable when comparing consecutive autoland pushes, because there's only one data point from each, and these tests have noise. A potential way around this is to retrigger the wa jobs on the pushes until the confidence values go up. Alternatively you can look at the graphs to see history over many pushes, it's often clear when there's a jump.

Can you point me to instructions to run these locally, so I can run them under a profiler and get a better understanding of what's happening?

These tests can be run by navigating to third_party/webkit/PerformanceTests/webaudio/index.html?raptor although you may need to serve that from local HTTP if it doesn't work over the file: protocol.

Flags: needinfo?(dmajor) → needinfo?(sgiesecke)
Regressions: 1644403
Flags: needinfo?(sgiesecke)
You need to log in before you can comment on or make changes to this bug.