1464428 - Optimize QuerySelector in shadow trees.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

Splitting this off bug 1426493.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1464428: Optimize QuerySelector in shadow trees.

Pretty much the same setup we have for document.

We have the awkwardness of having to check containing shadow manually for
ShadowRoot because it's not available in TNode (and making it available added a
bit more complexity that wasn't worth it IMO).

Review commit: https://reviewboard.mozilla.org/r/246770/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246770/

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8980622 [details]
Bug 1464428: Optimize QuerySelector in shadow trees.

https://reviewboard.mozilla.org/r/246770/#review253190

Look good. r=me with the comment updated.

::: servo/components/style/dom.rs:142
(Diff revision 1)
> -    fn elements_with_id(
> +    fn elements_with_id<'a>(
>          &self,
>          _id: &Atom,
> -    ) -> Result<&[<Self::ConcreteNode as TNode>::ConcreteElement], ()> {
> +    ) -> Result<&'a [<Self::ConcreteNode as TNode>::ConcreteElement], ()>
> +    where
> +        Self: 'a,

I'm wondering why we cannot do:
```rust
fn elements_with_id<'a>(
    &'a self,
    _id: &Atom,
) -> Result<'a ..., ()> {
    ...
}
```

::: servo/components/style/dom_apis.rs:263
(Diff revision 1)
>  /// Fast path for iterating over every element with a given id in the document
>  /// that `root` is connected to.

The comment should be updated, probably "in a document or shadow root that `root` is connected to"?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #2)
> Comment on attachment 8980622 [details]
> Bug 1464428: Optimize QuerySelector in shadow trees.
> 
> https://reviewboard.mozilla.org/r/246770/#review253190
> 
> Look good. r=me with the comment updated.
> 
> ::: servo/components/style/dom.rs:142
> (Diff revision 1)
> > -    fn elements_with_id(
> > +    fn elements_with_id<'a>(
> >          &self,
> >          _id: &Atom,
> > -    ) -> Result<&[<Self::ConcreteNode as TNode>::ConcreteElement], ()> {
> > +    ) -> Result<&'a [<Self::ConcreteNode as TNode>::ConcreteElement], ()>
> > +    where
> > +        Self: 'a,
> 
> I'm wondering why we cannot do:
> ```rust
> fn elements_with_id<'a>(
>     &'a self,
>     _id: &Atom,
> ) -> Result<'a ..., ()> {
>     ...
> }
> ```

That is because &self would refer to the current element / document / shadowdom, which is copy. So we need to explicitly tell that the array is valid for the lifetime of the DOM wrapper ('ld / 'le) instead.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8fb0fa2dca36
Optimize QuerySelector in shadow trees. r=xidorn

Comment 5

[Arthur Iakab [arthur_iakab]]

Backed out changeset 8fb0fa2dca36 (bug 1464428) for Reftest crashes on a CLOSED TREE
Backout revision https://hg.mozilla.org/integration/mozilla-inbound/rev/046f5a3c578243d3df738b05e3eda361d3809409
Failed push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8fb0fa2dca3622c96929d1b5807e130c28bb75ed&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Link to the log:https://treeherder.mozilla.org/logviewer.html#?job_id=180528488&repo=mozilla-inbound
Part of that log: 09:33:30     INFO - retry: Calling run_command with args: (['c:\\mozilla-build\\python\\python.exe', '-u', 'C:\\Users\\task_1527499542\\mozharness\\external_tools\\tooltool.py', '--url', 'https://tooltool.mozilla-releng.net/', 'fetch', '-m', 'C:\\Users\\task_1527499542\\build\\tests\\config/tooltool-manifests/win32/nodejs.manifest', '-o'],), kwargs: {'output_timeout': 600, 'error_list': [{'substr': 'command not found', 'level': 'error'}, {'regex': <_sre.SRE_Pattern object at 0x0000000003320928>, 'level': 'warning'}, {'substr': 'Traceback (most recent call last)', 'level': 'error'}, {'substr': 'SyntaxError: ', 'level': 'error'}, {'substr': 'TypeError: ', 'level': 'error'}, {'substr': 'NameError: ', 'level': 'error'}, {'substr': 'ZeroDivisionError: ', 'level': 'error'}, {'regex': <_sre.SRE_Pattern object at 0x0000000003315DA0>, 'level': 'critical'}, {'regex': <_sre.SRE_Pattern object at 0x0000000003373D98>, 'level': 'critical'}, {'substr': 'ERROR - ', 'level': 'error'}], 'cwd': 'C:\\Users\\task_1527499542\\build', 'privileged': False}, attempt #1
09:33:30     INFO - Running command: ['c:\\mozilla-build\\python\\python.exe', '-u', 'C:\\Users\\task_1527499542\\mozharness\\external_tools\\tooltool.py', '--url', 'https://tooltool.mozilla-releng.net/', 'fetch', '-m', 'C:\\Users\\task_1527499542\\build\\tests\\config/tooltool-manifests/win32/nodejs.manifest', '-o'] in C:\Users\task_1527499542\build
09:33:30     INFO - Copy/paste: c:\mozilla-build\python\python.exe -u C:\Users\task_1527499542\mozharness\external_tools\tooltool.py --url https://tooltool.mozilla-releng.net/ fetch -m C:\Users\task_1527499542\build\tests\config/tooltool-manifests/win32/nodejs.manifest -o
09:33:30     INFO - Calling ['c:\\mozilla-build\\python\\python.exe', '-u', 'C:\\Users\\task_1527499542\\mozharness\\external_tools\\tooltool.py', '--url', 'https://tooltool.mozilla-releng.net/', 'fetch', '-m', 'C:\\Users\\task_1527499542\\build\\tests\\config/tooltool-manifests/win32/nodejs.manifest', '-o'] with output_timeout 600
09:33:30     INFO -  INFO - Attempting to fetch from 'https://tooltool.mozilla-releng.net/'...
09:33:33     INFO -  INFO - File node-win32.exe fetched from https://tooltool.mozilla-releng.net/ as C:\Users\task_1527499542\build\tmplpzwhf
09:33:34     INFO -  INFO - File integrity verified, renaming tmplpzwhf to node-win32.exe
09:33:34     INFO - Return code: 0
09:33:34     INFO - ENV: MOZ_WEBRENDER is now 1
09:33:34     INFO - ENV: MOZ_NODE_PATH is now C:\Users\task_1527499542\build\node-win32.exe
09:33:34     INFO - ENV: STYLO_THREADS is now 4
09:33:34     INFO - ENV: RUST_BACKTRACE is now full
09:33:34     INFO - ENV: MOZ_UPLOAD_DIR is now C:\Users\task_1527499542\build\blobber_upload_dir
09:33:34     INFO - ENV: MOZ_ACCELERATED is now 1
09:33:34     INFO - ENV: MINIDUMP_STACKWALK is now C:\Users\task_1527499542\build\win32-minidump_stackwalk.exe
09:33:34     INFO - ENV: MINIDUMP_SAVE_PATH is now C:\Users\task_1527499542\build\blobber_upload_dir
09:33:34     INFO - Running command: ['C:\\Users\\task_1527499542\\build\\venv\\Scripts\\python', '-u', 'C:\\Users\\task_1527499542\\build\\tests\\reftest\\runreftest.py', '--total-chunks', '2', '--this-chunk', '1', '--appname=C:\\Users\\task_1527499542\\build\\application\\firefox\\firefox.exe', '--utility-path=tests/bin', '--extra-profile-file=tests/bin/plugins', '--symbols-path=https://queue.taskcluster.net/v1/task/XUC3LfEXRP6MFwWbMm-t2w/artifacts/public/build/target.crashreporter-symbols.zip', '--log-raw=C:\\Users\\task_1527499542\\build\\blobber_upload_dir\\reftest_raw.log', '--log-errorsummary=C:\\Users\\task_1527499542\\build\\blobber_upload_dir\\reftest_errorsummary.log', '--cleanup-crashes', '--marionette-startup-timeout=180', '--sandbox-read-whitelist=C:\\Users\\task_1527499542\\build', '--suite=reftest', '--', 'tests/reftest/tests/layout/reftests/reftest.list'] in C:\Users\task_1527499542\build
09:33:34     INFO - Copy/paste: C:\Users\task_1527499542\build\venv\Scripts\python -u C:\Users\task_1527499542\build\tests\reftest\runreftest.py --total-chunks 2 --this-chunk 1 --appname=C:\Users\task_1527499542\build\application\firefox\firefox.exe --utility-path=tests/bin --extra-profile-file=tests/bin/plugins --symbols-path=https://queue.taskcluster.net/v1/task/XUC3LfEXRP6MFwWbMm-t2w/artifacts/public/build/target.crashreporter-symbols.zip --log-raw=C:\Users\task_1527499542\build\blobber_upload_dir\reftest_raw.log --log-errorsummary=C:\Users\task_1527499542\build\blobber_upload_dir\reftest_errorsummary.log --cleanup-crashes --marionette-startup-timeout=180 --sandbox-read-whitelist=C:\Users\task_1527499542\build --suite=reftest -- tests/reftest/tests/layout/reftests/reftest.list
09:33:34     INFO - Using env: {'ACCESS_TOKEN': 'IGYofFzPSteBPdPmyISVHg',
09:33:34     INFO -  'ALLUSERSPROFILE': 'C:\\ProgramData',
09:33:34     INFO -  'APPDATA': 'C:\\Users\\GenericWorker\\AppData\\Roaming',
09:33:34     INFO -  'COMMONPROGRAMFILES': 'C:\\Program Files\\Common Files',
09:33:34     INFO -  'COMMONPROGRAMFILES(X86)': 'C:\\Program Files (x86)\\Common Files',
09:33:34     INFO -  'COMMONPROGRAMW6432': 'C:\\Program Files\\Common Files',
09:33:34     INFO -  'COMPUTERNAME': 'T-W1064-MS-115',
09:33:34     INFO -  'COMSPEC': 'C:\\windows\\system32\\cmd.exe',
09:33:34     INFO -  'GECKO_HEAD_REPOSITORY': 'https://hg.mozilla.org/integration/mozilla-inbound',
09:33:34     INFO -  'GECKO_HEAD_REV': '8fb0fa2dca3622c96929d1b5807e130c28bb75ed',
09:33:34     INFO -  'GPU_NAME': 'Intel(R) Iris(R) Pro Graphics P580  \r',
09:33:34     INFO -  'HOMEDRIVE': 'C:',
09:33:34     INFO -  'HOMEPATH': '\\Users\\GenericWorker',
09:33:34     INFO -  'LIVELOG_GET_PORT': '60023',
09:33:34     INFO -  'LIVELOG_PUT_PORT': '60022',
09:33:34     INFO -  'LOCALAPPDATA': 'C:\\Users\\GenericWorker\\AppData\\Local',
09:33:34     INFO -  'LOGONSERVER': '\\\\T-W1064-MS-115',
09:33:34     INFO -  'MINIDUMP_SAVE_PATH': 'C:\\Users\\task_1527499542\\build\\blobber_upload_dir',
09:33:34     INFO -  'MINIDUMP_STACKWALK': 'C:\\Users\\task_1527499542\\build\\win32-minidump_stackwalk.exe',
09:33:34     INFO -  'MOZILLABUILD': 'C:\\mozilla-build',
09:33:34     INFO -  'MOZ_ACCELERATED': '1',
09:33:34     INFO -  'MOZ_AUTOMATION': '1',
09:33:34     INFO -  'MOZ_NODE_PATH': 'C:\\Users\\task_1527499542\\build\\node-win32.exe',
09:33:34     INFO -  'MOZ_UPLOAD_DIR': 'C:\\Users\\task_1527499542\\build\\blobber_upload_dir',
09:33:34     INFO -  'MOZ_WEBRENDER': '1',
09:33:34     INFO -  'NUMBER_OF_PROCESSORS': '8',
09:33:34     INFO -  'ONEDRIVE': 'C:\\Users\\GenericWorker\\OneDrive',
09:33:34     INFO -  'OS': 'Windows_NT',
09:33:34     INFO -  'PATH': 'C:\\Users\\task_1527499542\\build\\venv\\Scripts;C:\\Program Files\\Mercurial;C:\\mozilla-build\\7zip;C:\\mozilla-build\\info-zip;C:\\mozilla-build\\kdiff3;C:\\mozilla-build\\moztools-x64\\bin;C:\\mozilla-build\\mozmake;C:\\mozilla-build\\msys\\bin;C:\\mozilla-build\\msys\\local\\bin;C:\\mozilla-build\\nsis-3.01;C:\\mozilla-build\\python;C:\\mozilla-build\\python\\Scripts;C:\\mozilla-build\\upx394w;C:\\mozilla-build\\wget;C:\\mozilla-build\\yasm;C:\\windows\\system32;C:\\windows;C:\\windows\\System32\\Wbem;C:\\windows\\System32\\WindowsPowerShell\\v1.0\\;C:\\Program Files (x86)\\GNU\\GnuPG\\pub;C:\\Program Files (x86)\\Windows Kits\\8.1\\Windows Performance Toolkit\\;C:\\Program Files\\Mercurial\\;C:\\windows\\system32\\config\\systemprofile\\AppData\\Local\\Microsoft\\WindowsApps;C:\\Users\\GenericWorker\\AppData\\Local\\Microsoft\\WindowsApps;C:\\Users\\task_1527499542\\build\\venv\\Lib\\site-packages\\pywin32_system32',
09:33:34     INFO -  'PATHEXT': '.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC',
09:33:34     INFO -  'PIP_DOWNLOAD_CACHE': 'c:\\pip-cache',
09:33:34     INFO -  'PROCESSOR_ARCHITECTURE': 'AMD64',
09:33:34     INFO -  'PROCESSOR_IDENTIFIER': 'Intel64 Family 6 Model 94 Stepping 3, GenuineIntel',
09:33:34     INFO -  'PROCESSOR_LEVEL': '6',
09:33:34     INFO -  'PROCESSOR_REVISION': '5e03',
09:33:34     INFO -  'PROGRAMDATA': 'C:\\ProgramData',
09:33:34     INFO -  'PROGRAMFILES': 'C:\\Program Files',
09:33:34     INFO -  'PROGRAMFILES(X86)': 'C:\\Program Files (x86)',
09:33:34     INFO -  'PROGRAMW6432': 'C:\\Program Files',
09:33:34     INFO -  'PROMPT': '$P$G',
09:33:34     INFO -  'PSMODULEPATH': '%ProgramFiles%\\WindowsPowerShell\\Modules;C:\\windows\\system32\\WindowsPowerShell\\v1.0\\Modules',
09:33:34     INFO -  'PUBLIC': 'C:\\Users\\Public',
09:33:34     INFO -  'RUST_BACKTRACE': 'full',
09:33:34     INFO -  'SCCACHE_DISABLE': '1',
09:33:34     INFO -  'SERVER_CRT_FILE': 'C:\\generic-worker\\livelog.crt',
09:33:34     INFO -  'SERVER_KEY_FILE': 'C:\\generic-worker\\livelog.key',
09:33:34     INFO -  'STYLO_THREADS': '4',
09:33:34     INFO -  'SYSTEMDRIVE': 'C:',
09:33:34     INFO -  'SYSTEMROOT': 'C:\\windows',
09:33:34     INFO -  'TASK_ID': 'ORehqJd3Rr6p3jQ1XQGzrw',
09:33:34     INFO -  'TEMP': 'C:\\Users\\GenericWorker\\AppData\\Local\\Temp',
09:33:34     INFO -  'TMP': 'C:\\Users\\GenericWorker\\AppData\\Local\\Temp',
09:33:34     INFO -  'TOOLTOOL_CACHE': 'c:\\tooltool-cache',
09:33:34     INFO -  'USERDOMAIN': 'T-W1064-MS-115',
09:33:34     INFO -  'USERDOMAIN_ROAMINGPROFILE': 'T-W1064-MS-115',
09:33:34     INFO -  'USERNAME': 'GenericWorker',
09:33:34     INFO -  'USERPROFILE': 'C:\\Users\\GenericWorker',
09:33:34     INFO -  'VS140COMNTOOLS': 'C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\Common7\\Tools\\',
09:33:34     INFO -  'WINDIR': 'C:\\windows'}
09:33:34     INFO - Calling ['C:\\Users\\task_1527499542\\build\\venv\\Scripts\\python', '-u', 'C:\\Users\\task_1527499542\\build\\tests\\reftest\\runreftest.py', '--total-chunks', '2', '--this-chunk', '1', '--appname=C:\\Users\\task_1527499542\\build\\application\\firefox\\firefox.exe', '--utility-path=tests/bin', '--extra-profile-file=tests/bin/plugins', '--symbols-path=https://queue.taskcluster.net/v1/task/XUC3LfEXRP6MFwWbMm-t2w/artifacts/public/build/target.crashreporter-symbols.zip', '--log-raw=C:\\Users\\task_1527499542\\build\\blobber_upload_dir\\reftest_raw.log', '--log-errorsummary=C:\\Users\\task_1527499542\\build\\blobber_upload_dir\\reftest_errorsummary.log', '--cleanup-crashes', '--marionette-startup-timeout=180', '--sandbox-read-whitelist=C:\\Users\\task_1527499542\\build', '--suite=reftest', '--', 'tests/reftest/tests/layout/reftests/reftest.list'] with output_timeout 1000
09:33:34     INFO -  REFTEST INFO | Removed pending crash reports at 'C:\Users\GenericWorker\AppData\Roaming\Mozilla\Firefox\Crash Reports'
09:33:34     INFO -  REFTEST INFO | Running with e10s: True
09:33:34     INFO -  REFTEST INFO | Application command: C:\Users\task_1527499542\build\application\firefox\firefox.exe -marionette -profile c:\users\genericworker\appdata\local\temp\tmpdf0p2y.mozrunner
09:33:35     INFO -  1527500015492	Marionette	DEBUG	Received observer notification profile-after-change
09:33:35     INFO -  1527500015575	Marionette	DEBUG	Received observer notification command-line-startup
09:33:35     INFO -  1527500015575	Marionette	DEBUG	Received observer notification nsPref:changed
09:33:35     INFO -  1527500015575	Marionette	DEBUG	Init aborted (running=false, enabled=true, finalUIStartup=false)
09:33:35     INFO -  WARN: rx::HLSLCompiler::compileToBinary(228):
09:33:35     INFO -  C:\fakepath(559,20-34): warning X3556: integer modulus may be much slower, try using uints if possible.
09:33:35     INFO -  C:\fakepath(559,39-53): warning X3556: integer divides may be much slower, try using uints if possible.
09:33:36     INFO -  WARN: rx::HLSLCompiler::compileToBinary(228):
09:33:36     INFO -  C:\fakepath(695,20-34): warning X3556: integer modulus may be much slower, try using uints if possible.
09:33:36     INFO -  C:\fakepath(695,39-53): warning X3556: integer divides may be much slower, try using uints if possible.
09:36:34  WARNING -  TEST-UNEXPECTED-FAIL | None | application terminated with exit code 1
09:36:34     INFO -  REFTEST INFO | Downloading symbols from: https://queue.taskcluster.net/v1/task/XUC3LfEXRP6MFwWbMm-t2w/artifacts/public/build/target.crashreporter-symbols.zip
09:36:40     INFO -  REFTEST INFO | Copy/paste: C:\Users\task_1527499542\build\win32-minidump_stackwalk.exe c:\users\genericworker\appdata\local\temp\tmpdf0p2y.mozrunner\minidumps\b3f4392c-3bcd-4ce5-bd6a-4ba5c6c0e938.dmp c:\users\genericworker\appdata\local\temp\tmpahtwav
09:36:43     INFO -  REFTEST INFO | Saved minidump as C:\Users\task_1527499542\build\blobber_upload_dir\b3f4392c-3bcd-4ce5-bd6a-4ba5c6c0e938.dmp
09:36:43     INFO -  REFTEST INFO | Saved app info as C:\Users\task_1527499542\build\blobber_upload_dir\b3f4392c-3bcd-4ce5-bd6a-4ba5c6c0e938.extra
09:36:43    ERROR -  REFTEST PROCESS-CRASH | pid: None | application crashed [@ PLDHashTable::Search(void const *)]
09:36:43     INFO -  Crash dump filename: c:\users\genericworker\appdata\local\temp\tmpdf0p2y.mozrunner\minidumps\b3f4392c-3bcd-4ce5-bd6a-4ba5c6c0e938.dmp

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d53c55e96df
Optimize QuerySelector in shadow trees. r=xidorn

Comment 7

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/1d53c55e96df

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

this patch causes a perma fail for talos:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=win%20talos&tochange=8fb0fa2dca3622c96929d1b5807e130c28bb75ed&fromchange=48a5d87cf9bdb59e39653ab331df04ea2a04267d

which appears to be fixed by the new version.