Bug 1331674 (SyncIPC)

[meta] Stop doing sync IPC from the content process or the UI thread

NEW
Unassigned

Status

()

defect
P3
normal
3 years ago
3 months ago

People

(Reporter: Ehsan, Unassigned)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qf:meta], )

Reporter

Description

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

Updated

3 years ago
Depends on: 1331676
Reporter

Updated

3 years ago
Depends on: 1303749, 1268335
Depends on: 1303096
Reporter

Updated

3 years ago
Depends on: 1331680
Reporter

Updated

3 years ago
Depends on: 1331684
Reporter

Updated

3 years ago
Depends on: 1317322
Reporter

Updated

3 years ago
See Also: → 1296160
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

3 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

3 years ago
Depends on: 1330912
Reporter

Updated

3 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: Quantum
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: QuantumFlow
Reporter

Updated

2 years ago
No longer blocks: Quantum
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] → [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

2 years ago
Depends on: 1375022
Reporter

Updated

2 years ago
Depends on: 1345058
Reporter

Updated

2 years ago
Depends on: 1393909
Reporter

Updated

2 years ago
Depends on: 1395813
Reporter

Updated

2 years ago
Depends on: 1395814
Reporter

Updated

2 years ago
Depends on: 1395815
Sorry, should already have marked these meta bugs as P3.
Priority: -- → P3
Keywords: perf

Updated

Last year
Depends on: 1349699
Depends on: 1503491
Summary: Stop doing sync IPC from the content process or the UI thread → [meta] Stop doing sync IPC from the content process or the UI thread
Depends on: 1528665
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.