Open
Bug 1169208
Opened 10 years ago
Updated 2 years ago
Windows low integrity temp set-up and tear-down causes T-e10s(x) failures.
Categories
(Core :: Security: Process Sandboxing, defect)
Tracking
()
ASSIGNED
People
(Reporter: bobowen, Assigned: bobowen)
References
(Blocks 1 open bug)
Details
(Whiteboard: sb+)
Bug 1162327 added main thread I/O for the set-up and tear-down of the low integrity temp directory.
This only happens for e10s.
The start-up I/O certainly needs to be moved off main thread.
If the shutdown I/O stays on main thread, then we need to whitelist it in the talos tests.
Comment 1•10 years ago
|
||
The low integrity temp directory is created by the content process, right? Is creating the temp directory on the critical initialization path? i.e. would performance actually benefit from moving this operation off the content-process main thread?
Flags: needinfo?(bobowen.code)
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #1)
> The low integrity temp directory is created by the content process, right?
> Is creating the temp directory on the critical initialization path? i.e.
> would performance actually benefit from moving this operation off the
> content-process main thread?
It used to be created in the content process, but now it is created in the chrome process on start-up.
This is because it is now the same for all content processes (per profile), to make it easier to clean-up.
I could move it back to the content process and, if multi-content process is turned on, leave it up to the OS to handle concurrency.
That or submit the creation to the I/O thread in the main process.
I think I'll still need to do the deletion in the main process either way.
Flags: needinfo?(bobowen.code)
Updated•9 years ago
|
tracking-e10s:
--- → -
Comment 3•9 years ago
|
||
I'm still not 100% clear on whether moving this startup I/O off the main thread will help performance
If you're going to try improving performance by moving the I/O off the main thread, I'd suggest having the parent process I/O thread do this initialization. Do Talos try pushes with and without the optimization to get a sense for whether it actually improved (warm) startups. You can compare the Talos results from two try pushes using this tool https://treeherder.mozilla.org/perf.html#/comparechooser
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #3)
> I'm still not 100% clear on whether moving this startup I/O off the main
> thread will help performance
>
> If you're going to try improving performance by moving the I/O off the main
> thread, I'd suggest having the parent process I/O thread do this
> initialization. Do Talos try pushes with and without the optimization to get
> a sense for whether it actually improved (warm) startups. You can compare
> the Talos results from two try pushes using this tool
> https://treeherder.mozilla.org/perf.html#/comparechooser
Thanks Vladan.
I should get to this soon...ish.
I think that the I/O that is being caught is only the shutdown I/O.
This is because the particular test (mainthreadio.py) that is failing doesn't track start-up I/O.
I think I'll need to modify that test to allow for wild cards in the directory name.
I wonder why the other I/O test (xperf?), isn't catching the start-up I/O.
Either way I'll test as you advise, to see if it makes a difference.
Comment 5•9 years ago
|
||
(In reply to Bob Owen (:bobowen) from comment #4)
> I wonder why the other I/O test (xperf?), isn't catching the start-up I/O.
Hmm, I wonder if it's because it's not in a monitored directory?
Aaron, can you tell us whether the xperf failures for file '{appdata}\locallow\mozilla\temp-...} in the link below are caused by startup I/O or shutdown I/O?
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=xperf&exclusion_profile=false
Flags: needinfo?(aklotz)
Comment 6•9 years ago
|
||
(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #5)
> (In reply to Bob Owen (:bobowen) from comment #4)
> > I wonder why the other I/O test (xperf?), isn't catching the start-up I/O.
>
> Hmm, I wonder if it's because it's not in a monitored directory?
>
> Aaron, can you tell us whether the xperf failures for file
> '{appdata}\locallow\mozilla\temp-...} in the link below are caused by
> startup I/O or shutdown I/O?
>
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-
> searchStr=xperf&exclusion_profile=false
Apparently we aren't uploading the .etl file when the test goes orange, so I don't have that information. I'm filing a bug about that.
Flags: needinfo?(aklotz)
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #6)
> (In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #5)
> > (In reply to Bob Owen (:bobowen) from comment #4)
> > > I wonder why the other I/O test (xperf?), isn't catching the start-up I/O.
> >
> > Hmm, I wonder if it's because it's not in a monitored directory?
> >
> > Aaron, can you tell us whether the xperf failures for file
> > '{appdata}\locallow\mozilla\temp-...} in the link below are caused by
> > startup I/O or shutdown I/O?
> >
> > https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-
> > searchStr=xperf&exclusion_profile=false
>
> Apparently we aren't uploading the .etl file when the test goes orange, so I
> don't have that information. I'm filing a bug about that.
I finally got back to this and managed to get this running on Talos with e10s.
I think I've run the tests that involve start-up a few times (although the docs aren't very clear).
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=dab65ddd67d6&newProject=try&newRevision=b01fcb185a5d
As far as I can tell, it doesn't seem to have made a significant difference.
Obviously, I have a patch for doing the creation on the IO Thread now, do you think it is worth trying to land it?
Did you get anywhere over the .etl file and why this new start-up file access isn't getting caught?
I'll work on a patch for the mainthreadio test to allow us to add some sort of wild-card exceptions for the shutdown access.
I thought the orange due to shutdown access had magically gone away, but when I investigated a change had meant that the whole test was failing silently (bug 1178742).
Flags: needinfo?(aklotz)
Comment 9•9 years ago
|
||
is bug 1171129 blocking this? I can probably get that fixed up.
Comment 10•9 years ago
|
||
xperf e10s is now working, I am fine calling this worksforme.
Updated•9 years ago
|
Flags: needinfo?(aklotz)
Comment 11•9 years ago
|
||
The talos 'x' suite can fail two different ways:
A xperf failure will indicate startup main-thread I/O.
A mainthreadio.py failure will be non-startup main-thread I/O, including shutdown.
Whitelisting should depend on whether this I/O impacts perf in practice. I'd suggest running Nightly on a magnetic hdd with a flushed page cache while procmon or xperf is running to see how expensive the I/O is.
Flags: needinfo?(aklotz)
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•