Bug 1331674 (SyncIPC)

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

NEW
Unassigned

Status

()

P3
normal
2 years ago
2 months ago

People

(Reporter: Ehsan, Unassigned)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qf:meta], URL)

(Reporter)

Description

2 years ago
This bug tracks the perf issues caused by the content process doing sync IPC to the parent.
(Reporter)

Updated

2 years ago
Depends on: 1331676
(Reporter)

Updated

2 years ago
Depends on: 1303749, 1268335
Depends on: 1303096
(Reporter)

Updated

2 years ago
Depends on: 1331680
(Reporter)

Updated

2 years ago
Depends on: 1331684
(Reporter)

Updated

2 years ago
Depends on: 1317322
(Reporter)

Updated

2 years ago
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)
(Reporter)

Comment 2

2 years ago
(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)
(Reporter)

Updated

2 years ago
Depends on: 1330912
(Reporter)

Updated

2 years ago
Depends on: 1332036
Keywords: meta
(Reporter)

Updated

2 years ago
Depends on: 1333600
(Reporter)

Comment 3

2 years ago
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
(Reporter)

Updated

2 years ago
Blocks: 1325169
(Reporter)

Comment 4

2 years ago
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.
(Reporter)

Comment 6

2 years ago
(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.
and for JS side then sendSyncMessage
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.
(Reporter)

Comment 11

2 years ago
(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.
(Reporter)

Updated

2 years ago
Depends on: 1337056
(Reporter)

Updated

2 years ago
Depends on: 1337058
(Reporter)

Updated

2 years ago
Depends on: 1337062
(Reporter)

Updated

2 years ago
Depends on: 1337063
(Reporter)

Updated

2 years ago
Depends on: 1337064
(Reporter)

Updated

2 years ago
Depends on: 1337077
(Reporter)

Updated

2 years ago
Depends on: 1337080
(Reporter)

Updated

2 years ago
Blocks: 1337841
(Reporter)

Updated

2 years ago
No longer blocks: 1325169
(Reporter)

Updated

2 years ago
Depends on: 1339543
(Reporter)

Updated

2 years ago
Depends on: 1340573
Depends on: 1340748
(Reporter)

Updated

2 years ago
Depends on: 1342333
(Reporter)

Updated

2 years ago
Depends on: 1342632
(Reporter)

Updated

2 years ago
Depends on: 1342635
(Reporter)

Updated

2 years ago
Depends on: 1342636
Depends on: 1343383
(Reporter)

Updated

2 years ago
Depends on: 1343728
(Reporter)

Updated

2 years ago
Depends on: 1343729
(Reporter)

Updated

2 years ago
Depends on: 1343731
(Reporter)

Updated

2 years ago
Depends on: 1344216
Whiteboard: qf:meta]
Whiteboard: qf:meta] → [qf:meta]
(Reporter)

Updated

2 years ago
Depends on: 1346583
(Reporter)

Updated

2 years ago
Depends on: 1347035
(Reporter)

Updated

2 years ago
Depends on: 1347425
(Reporter)

Updated

2 years ago
Depends on: 1348113
(Reporter)

Updated

2 years ago
Depends on: 1348613
(Reporter)

Updated

2 years ago
Depends on: 1349255
(Reporter)

Updated

2 years ago
Depends on: 1349696
(Reporter)

Updated

2 years ago
Depends on: 1350633
(Reporter)

Updated

2 years ago
Depends on: 1350634
(Reporter)

Updated

2 years ago
Depends on: 1350635
(Reporter)

Updated

2 years ago
Depends on: 1350636
(Reporter)

Updated

2 years ago
Depends on: 1350637
(Reporter)

Updated

2 years ago
Depends on: 1350638
(Reporter)

Updated

2 years ago
Depends on: 1350640
(Reporter)

Updated

2 years ago
Depends on: 1350642
(Reporter)

Updated

2 years ago
Depends on: 1350643
(Reporter)

Updated

2 years ago
Depends on: 1350644
Depends on: 1351690
Depends on: 1352218
No longer depends on: 1351690

Updated

2 years ago
Depends on: 1353029
Depends on: 1354641
(Reporter)

Updated

2 years ago
Depends on: 1360171
(Reporter)

Updated

2 years ago
Depends on: 1360406
(Reporter)

Updated

2 years ago
Depends on: 1365697
(Reporter)

Updated

2 years ago
Depends on: 1365719
(Reporter)

Updated

2 years ago
Depends on: 1371523
(Reporter)

Updated

2 years ago
Depends on: 1371525
Depends on: 1373773
(Reporter)

Updated

a year ago
Depends on: 1375022
(Reporter)

Updated

a year ago
Depends on: 1345058
(Reporter)

Updated

a year ago
Depends on: 1393909
(Reporter)

Updated

a year ago
Depends on: 1395813
(Reporter)

Updated

a year ago
Depends on: 1395814
(Reporter)

Updated

a year ago
Depends on: 1395815
Sorry, should already have marked these meta bugs as P3.
Priority: -- → P3

Updated

8 months ago
Depends on: 1349699
You need to log in before you can comment on or make changes to this bug.