Bug 1331674 (SyncIPC)

Stop doing sync IPC from the content process or the UI thread

NEW
Unassigned

Status

()

Core
DOM
7 months ago
27 days ago

People

(Reporter: Ehsan, Unassigned)

Tracking

(Depends on: 17 bugs, Blocks: 1 bug, {meta})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qf:meta], URL)

This bug tracks the perf issues caused by the content process doing sync IPC to the parent.
Depends on: 1331676
Depends on: 1303749, 1268335
Depends on: 1303096
Depends on: 1331680
Depends on: 1331684
Depends on: 1317322
See Also: → bug 1296160
Depends on: 1194751
Hey ehsan,

Thanks for organizing these bugs! Is there a concerted effort being started here? Or is this more about gathering bugs together for now?
Flags: needinfo?(ehsan)
(In reply to Mike Conley (:mconley) - Getting through review / needinfo backlog from comment #1)
> Hey ehsan,
> 
> Thanks for organizing these bugs! Is there a concerted effort being started
> here?

Not yet, but that may change...  For now I just want a place to look for tracking this stuff.
Flags: needinfo?(ehsan)
Depends on: 1330912
Depends on: 1332036

Updated

7 months ago
Depends on: 1333489
Keywords: meta
Depends on: 1333600
I'm extending this bug to also track sync IPCs from the UI thread, like bug 1334655.
Depends on: 1334655
Summary: Stop doing sync IPC from the content process → Stop doing sync IPC from the content process or the UI thread
Blocks: 1325169
As a final goal for this bug, I propose writing a patch to the IPDL compiler which would reject all sync IPC messages except for anything that we find where there's no other way to do things that we'll somehow annotate.

That way we won't have to keep fighting this issue forever!
(In reply to :Ehsan Akhgari from comment #4)
> As a final goal for this bug, I propose writing a patch to the IPDL compiler
> which would reject all sync IPC messages except for anything that we find
> where there's no other way to do things that we'll somehow annotate.
> 
> That way we won't have to keep fighting this issue forever!

What about doing this now? Adding the annotation to all the current ones and forbidding new sync messages to be created.
This way no new ones can be introduced while the currently existing ones are being fixed.
(In reply to Marco Castelluccio [:marco] from comment #5)
> (In reply to :Ehsan Akhgari from comment #4)
> > As a final goal for this bug, I propose writing a patch to the IPDL compiler
> > which would reject all sync IPC messages except for anything that we find
> > where there's no other way to do things that we'll somehow annotate.
> > 
> > That way we won't have to keep fighting this issue forever!
> 
> What about doing this now? Adding the annotation to all the current ones and
> forbidding new sync messages to be created.
> This way no new ones can be introduced while the currently existing ones are
> being fixed.

That's a fantastic idea!
Here's a list of all the sync messages, I think.

Comment 8

7 months ago
and for JS side then sendSyncMessage

Comment 9

7 months ago
I wonder if we could change the name of sendSyncMessage. Could it be sendSlowSyncMessage or something which warns the patch author and reviewer.
Depends on: 1336919
(In reply to Olli Pettay [:smaug] from comment #9)
> I wonder if we could change the name of sendSyncMessage. Could it be
> sendSlowSyncMessage or something which warns the patch author and reviewer.

Would it break addons? Otherwise it would be pretty straightforward to search & replace all the files.
(In reply to Kan-Ru Chen [:kanru] (UTC+8) from comment #10)
> (In reply to Olli Pettay [:smaug] from comment #9)
> > I wonder if we could change the name of sendSyncMessage. Could it be
> > sendSlowSyncMessage or something which warns the patch author and reviewer.
> 
> Would it break addons? Otherwise it would be pretty straightforward to
> search & replace all the files.

Yes, unfortunately this function is super popular in add-ons. :( 
https://dxr.mozilla.org/addons/search?q=sendSyncMessage&redirect=true
We could shim it, perhaps.
(In reply to Mike Conley (:mconley) from comment #12)
> We could shim it, perhaps.

Although that won't be useful for add-ons that are marked multiprocessCompatible - and these are the ones probably most likely to be using sendSyncMessage. :/ So nevermind.
Depends on: 1337056
Depends on: 1337058
Depends on: 1337062
Depends on: 1337063
Depends on: 1337064
Depends on: 1337077
Depends on: 1337080
Blocks: 1337841
No longer blocks: 1325169
Depends on: 1339543
Depends on: 1340573
Depends on: 1340748
Depends on: 1342333
Depends on: 1342632
Depends on: 1342635
Depends on: 1342636
Depends on: 1343383
Depends on: 1343728
Depends on: 1343729
Depends on: 1343731
Depends on: 1344216
Whiteboard: qf:meta]
Whiteboard: qf:meta] → [qf:meta]
Depends on: 1346583
Depends on: 1347035
Depends on: 1347425
Depends on: 1348113
Depends on: 1348613
Depends on: 1349255
Depends on: 1349696
Depends on: 1350633
Depends on: 1350634
Depends on: 1350635
Depends on: 1350636
Depends on: 1350637
Depends on: 1350638
Depends on: 1350640
Depends on: 1350642
Depends on: 1350643
Depends on: 1350644
Depends on: 1351690
Depends on: 1352218
No longer depends on: 1351690

Updated

5 months ago
Depends on: 1353029
Depends on: 1354641
Depends on: 1360171
Depends on: 1360406
Depends on: 1360736
Depends on: 1365697
Depends on: 1365719
Depends on: 1371523
Depends on: 1371525
Depends on: 1373773
Depends on: 1375022
Depends on: 1375243
Depends on: 1375244
Depends on: 1345058
You need to log in before you can comment on or make changes to this bug.