1194761 - PCookieService should probably not use sync messages

Status: RESOLVED DUPLICATE of bug 1331680

(Core :: DOM: Content Processes, defect, P4)

Description

[Mike Conley (:mconley) (:⚙️)]

I've got a profile that shows workday querying for cookie values off of a timer.

This uses sync messages, which causes the entire content process to lock up until it's serviced by the parent.

http://people.mozilla.org/~bgirard/cleopatra/#report=536d333e6a640766ee5e7fb388ad964a66b1dd08&filter=[{%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A484068,%22end%22%3A489501},{%22type%22%3A%22RangeSampleFilter%22,%22start%22%3A484495,%22end%22%3A485477}]&selection=0,5417,5418,5419,5420,5421,5422,8,9,10,11,12,13,14,15,16,17,20,21,22,23,24,25,26,199,200,201,340,341,342,343,344,128,129,151,152,153,128,129,130,131,132,133,5437,5710,7552,5712,5713,7553,5712,5715,7554,5527,159,128,129,266,129,130,131,132,133,5437,5717,7555,5712,5719,7556,5527,159,128,129,130,5721,131,161,132,133,5437,5722,7557,5527,159,128,129,130,5724,5725,131,161,132,133,5437,5726,7558,5712,5728,7559,5712,5730,5470,412,428,384,385,386,387,385,386,5508,2139,1296,419,424,128,129,1523,5731,5732,5733,5734,5735,461,462,463,464

It might be good if we can find a way of avoiding these messages if possible.

Comment 1

[Ting-Yu Chou [:ting] (away)]

I guess it has to be sync message as the content is accessing |document.cookie|. The best thing we can do is probably bug 1232638.

Comment 2

[Gabor Krizsanits (INACTIVE)]

As far as I know all the database related work and the rest we're waiting for here was done on the main thread with e10s off. Which means I would expect the same amount of time spent in the cookie getter methods blocking everything else as we do now in __psynch_cvwait. The chrome side of these hangs turned out not to be too significant, I wonder if we should really worry about the content process part of the hang. I would expect these two to be close to equal. Is there an STR to reproduce your experiment? Or could you also do a profiling for the non-e10s case?

Comment 3

[Mike Conley (:mconley) (:⚙️)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #2)
> As far as I know all the database related work and the rest we're waiting
> for here was done on the main thread with e10s off. Which means I would
> expect the same amount of time spent in the cookie getter methods blocking
> everything else as we do now in __psynch_cvwait. The chrome side of these
> hangs turned out not to be too significant, I wonder if we should really
> worry about the content process part of the hang. I would expect these two
> to be close to equal. Is there an STR to reproduce your experiment? Or could
> you also do a profiling for the non-e10s case?

I gathered this profile after experiencing a spinner while switching tabs, and it happened to point the finger at Workday, which I must have had open in one of my tabs.

I don't know how frequently Workday queries the cookie database - but it looks like, at least back when I gathered the profile, it did it off some kind of timer. I don't know how frequently that timer runs. I just opened Workday and sat there for a while and then attempted to gather a profile, and I didn't see anything. :/

So I don't think I have reliable STR here - although perhaps we could instrument setTimeout to figure out when Workday sets timeouts to fire for, and then try to manually trigger it somehow? I don't know - that might be a long-shot. The fact that Workday's source code is obfuscated really doesn't help.

Sorry I can't be more helpful here.

Comment 4

[Gabor Krizsanits (INACTIVE)]

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #3)
> So I don't think I have reliable STR here

I tried to reproduce it but all the times I catch a hang or just capture some cookie related messaging it seems quite insignificant:

https://cleopatra.io/#report=944153f2654867537c7c9d3bbc9848d88afde36c&jankOnly=true&invertCallback=true&selection=%22(total)%22,1552

https://cleopatra.io/#report=ac8fdeb129cf6b6a1a7efa1573d70280e87a2aef&jankOnly=true&invertCallback=true&selection=%22(total)%22,2638,2637,2636,4560,4559,4558,4557,4556,53,52,51,33,32,39,38,37,33,32,50,4192,7361,6097,4184,42,41,35,34,33,32,31,30,29,28,56,6096,6095,6094,6093,6092,6091,6090,6089,6088,6087,6086,6588,6587,4294,4293,4292,4291,4290,4285,4289

Even if they were significant I don't see how would it be a lot better without e10s. I think the only problem that can happen and what you witnessed is this: there is a chrome process hang for some reason (unrelated), and content process tries to get cookies and start to hang too. I think in that case the chrome process hang is to be fixed, and the side effect to hang the content process (rarely) as well is less important. I don't see any reason to keep this P1, what do you think Mike?

Comment 5

[Mike Conley (:mconley) (:⚙️)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #4)
> (In reply to Mike Conley (:mconley) - Needinfo me! from comment #3)
> > So I don't think I have reliable STR here
> 
> I tried to reproduce it but all the times I catch a hang or just capture
> some cookie related messaging it seems quite insignificant:
> 
> https://cleopatra.io/
> #report=944153f2654867537c7c9d3bbc9848d88afde36c&jankOnly=true&invertCallback
> =true&selection=%22(total)%22,1552
> 
> https://cleopatra.io/
> #report=ac8fdeb129cf6b6a1a7efa1573d70280e87a2aef&jankOnly=true&invertCallback
> =true&selection=%22(total)%22,2638,2637,2636,4560,4559,4558,4557,4556,53,52,
> 51,33,32,39,38,37,33,32,50,4192,7361,6097,4184,42,41,35,34,33,32,31,30,29,28,
> 56,6096,6095,6094,6093,6092,6091,6090,6089,6088,6087,6086,6588,6587,4294,
> 4293,4292,4291,4290,4285,4289
> 
> Even if they were significant I don't see how would it be a lot better
> without e10s. I think the only problem that can happen and what you
> witnessed is this: there is a chrome process hang for some reason
> (unrelated), and content process tries to get cookies and start to hang too.
> I think in that case the chrome process hang is to be fixed, and the side
> effect to hang the content process (rarely) as well is less important. I
> don't see any reason to keep this P1, what do you think Mike?

That sounds reasonable.

Comment 6

[Gabor Krizsanits (INACTIVE)]

After talking about this and the other PCookieService related bug with many people and we all agreed that this issue is not important for now I'm setting its priority back.