Closed Bug 1510490 Opened 5 years ago Closed 5 years ago

Use a hash digest rather than complete source to key shaders

Categories

(Core :: Graphics: WebRender, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: bholley, Assigned: bholley)

References

(Blocks 1 open bug)

Details

Attachments

(9 files)

Right now we keep the complete concatenated source of every shader around, which winds up costing us >10MB due to all the permutations. We discussed this, and decided to use a hash digest instead. I'm working on a patch.
Priority: -- → P2
Just pushing this down my patch queue to avoid triggering large cargo
rebuilds when rebasing.

Depends on D13438
The latter causes confusion in the memory reports because it gets summed
up and thus effectively doubles the reported texture memory usage. I've
decided it's best to drop, and so might as well do that while we're
already messing around with the memory reports and the associated
boilerplate.

Depends on D13439
This will need coordiland.
Flags: needinfo?(kats)
Up-to-date patches for this bug are also currently at https://github.com/bholley/gecko/commits/slice_r in case that's easier than scraping them out of Phabricator (I'm not sure if there's a way to pull phabricator revs or not).
Thanks for the heads up. I might actually just land the stuff from phabricator directly, along with an extra patch that updates the gfx/webrender_bindings/revision.txt. It seems like a shame to squash your nicely-split-up patchset. But having the GH branch is good - I haven't figured out a good way to pull stuff out of phabricator yet, so thanks for that link.

Also note that the treewide clang-format just landed so I'm not sure if your gecko-side patches need to be rebased on top of that.
Flags: needinfo?(kats)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11)
> Also note that the treewide clang-format just landed so I'm not sure if your
> gecko-side patches need to be rebased on top of that.

There were a couple of things that needed manual rebasing. Trivial changes but still something hg/lando can't do by itself. So I'll have to comandeer your revisions in Phabricator and push the rebased patches up, and then land them.
I can also just push a rebased version to phabricator and lando it myself if that's easier?
Yes please, that would be easier. Commandeering actually wouldn't work anyway. Feel free to land them now
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/96e1ddc0637b
Improve some comments. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/094544e620e1
Add dependency on SHA2. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/6fafd118a82a
Measure shader cache memory usage and remove total_gpu_bytes. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/c02d08e9dd38
Rejigger shader concatenation to avoid allocating and operate on a callback. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/ee215fdef53f
Use digests instead of sources to key cached binaries. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/55fa8c9b0c7e
Add versioning support for serialized shaders. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/f9d8e4ebe0a2
Cache exactly the shaders that are used early in startup. r=mattwoodrow,sotaro
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fedf8f5b79e5
Follow-up to fix Btup build bustage on a CLOSED TREE. r=kats
Backed out 7 changesets (Bug 1510490) for Btup bustages CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=f4cb63cf5e47447de99ca1d236696a3a63e09629&selectedJob=215022866

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=215022866&repo=autoland&lineNumber=1872

[task 2018-12-01T03:17:06.978Z] 03:17:06     INFO -       |                - un-closed delimiter
[task 2018-12-01T03:17:06.978Z] 03:17:06     INFO -  ...
[task 2018-12-01T03:17:06.979Z] 03:17:06     INFO -  2196 |     pub type U9007199254740992 = UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>
[task 2018-12-01T03:17:06.979Z] 03:17:06     INFO -       |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                      ^
[task 2018-12-01T03:17:06.979Z] 03:17:06     INFO -  error: expected one of `!`, `+`, `,`, `::`, or `>`, found `}`
[task 2018-12-01T03:17:06.979Z] 03:17:06     INFO -      --> /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/release/build/typenum-48f2d84349201725/out/consts.rs:2196:533
[task 2018-12-01T03:17:06.980Z] 03:17:06     INFO -       |
[task 2018-12-01T03:17:06.980Z] 03:17:06     INFO -  2196 |     pub type U9007199254740992 = UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>, B0>
[task 2018-12-01T03:17:06.980Z] 03:17:06     INFO -       |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                     ^ expected one of `!`, `+`, `,`, `::`, or `>` here
[task 2018-12-01T03:17:06.980Z] 03:17:06     INFO -  error: aborting due to 2 previous errors
[task 2018-12-01T03:17:06.981Z] 03:17:06     INFO -   *** tup messages ***
[task 2018-12-01T03:17:06.981Z] 03:17:06     INFO -   *** Command ID=358232 failed with return value 1
[task 2018-12-01T03:17:18.456Z] 03:17:18     INFO -   *** tup: 2 jobs failed.
[task 2018-12-01T03:17:18.537Z] 03:17:18     INFO -  0 compiler warnings present.
[task 2018-12-01T03:17:18.601Z] 03:17:18    ERROR - Return code: 1
[task 2018-12-01T03:17:18.601Z] 03:17:18  WARNING - setting return code to 2
[task 2018-12-01T03:17:18.601Z] 03:17:18    FATAL - 'mach build -v' did not run successfully. Please check log for errors.
[task 2018-12-01T03:17:18.601Z] 03:17:18    FATAL - Running post_fatal callback...
[task 2018-12-01T03:17:18.601Z] 03:17:18    FATAL - Exiting -1
[task 2018-12-01T03:17:18.602Z] 03:17:18     INFO - [mozharness: 2018-12-01 03:17:18.601975Z] Finished build step (failed)
[task 2018-12-01T03:17:18.602Z] 03:17:18     INFO - Running post-run listener: _shutdown_sccache
[task 2018-12-01T03:17:18.602Z] 03:17:18     INFO - Running command: ['/builds/worker/workspace/build/src/sccache2/sccache', '--stop-server'] in /builds/worker/workspace/build/src
[task 2018-12-01T03:17:18.602Z] 03:17:18     INFO - Copy/paste: /builds/worker/workspace/build/src/sccache2/sccache --stop-server
[task 2018-12-01T03:17:18.606Z] 03:17:18     INFO -  Stopping sccache server...
[task 2018-12-01T03:17:18.606Z] 03:17:18     INFO -  error: couldn't connect to server
[task 2018-12-01T03:17:18.606Z] 03:17:18     INFO -  caused by: Connection refused (os error 111)
[task 2018-12-01T03:17:18.606Z] 03:17:18    ERROR - Return code: 2
[task 2018-12-01T03:17:18.606Z] 03:17:18     INFO - Running post-run listener: _summarize
[task 2018-12-01T03:17:18.606Z] 03:17:18    ERROR - # TBPL FAILURE #
[task 2018-12-01T03:17:18.606Z] 03:17:18     INFO - [mozharness: 2018-12-01 03:17:18.606915Z] FxDesktopBuild summary:
[task 2018-12-01T03:17:18.607Z] 03:17:18    ERROR - # TBPL FAILURE #
[task 2018-12-01T03:17:18.613Z] cleanup
[task 2018-12-01T03:17:18.613Z] + cleanup
[task 2018-12-01T03:17:18.613Z] + local rv=255
[task 2018-12-01T03:17:18.613Z] + cleanup_xvfb
[task 2018-12-01T03:17:18.614Z] pidof Xvfb
[task 2018-12-01T03:17:18.614Z] ++ pidof Xvfb
[task 2018-12-01T03:17:18.615Z] + local xvfb_pid=55
[task 2018-12-01T03:17:18.615Z] + local vnc=false
[task 2018-12-01T03:17:18.615Z] + local interactive=false
[task 2018-12-01T03:17:18.615Z] + '[' -n 55 ']'
[task 2018-12-01T03:17:18.615Z] + [[ false == false ]]
[task 2018-12-01T03:17:18.615Z] + [[ false == false ]]
[task 2018-12-01T03:17:18.615Z] + kill 55
[task 2018-12-01T03:17:18.615Z] + screen -XS xvfb quit
[task 2018-12-01T03:17:18.749Z] No screen session found.
[task 2018-12-01T03:17:18.749Z] + true
[task 2018-12-01T03:17:18.749Z] + exit 255
[taskcluster 2018-12-01 03:17:19.089Z] === Task Finished ===
[taskcluster 2018-12-01 03:17:25.640Z] Unsuccessful task run with exit code: 255 completed in 337.41
Flags: needinfo?(bobbyholley)
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b425a0d640af
Backed out 7 changesets for Btup bustages CLOSED TREE
https://hg.mozilla.org/integration/autoland/rev/40e9134d4295
Backed out changeset fedf8f5b79e5 for not fixing the Btup bustages CLOSED TREE
:mshal, do you know what we need to go to get the tup build to pass? My attempt at listing the new generated files helped but didn't fix everything it seems.
Flags: needinfo?(mshal)
I strongly suspect this is because seems to assume the build script is called build.rs, which is not the case for typenum.

https://searchfox.org/mozilla-central/rev/3564dfde3b125878dc5a04fe92629fc5195942df/python/mozbuild/mozbuild/backend/tup.py#894

I'm experimenting with relaxing this restriction.
As this is blocking getting WR updates into m-c, I'd like to land the patch from the above try push, or something equivalent to get us unblocked. Not sure if anybody is around at the moment but I'll put the patch up anyway.
Running into https://bugzilla.mozilla.org/show_bug.cgi?id=1489126 trying to land this via Lando. I'll push it to inbound manually later when I'm back at a computer (on phone atm)
I'll push it to autoland (I have magic bits).
Flags: needinfo?(mshal)
Flags: needinfo?(bobbyholley)
Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40bf513c2e61
Improve some comments. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/96f2bb9d4598
Add dependency on SHA2. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/ef8f344dca81
Measure shader cache memory usage and remove total_gpu_bytes. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/783f72b27887
Rejigger shader concatenation to avoid allocating and operate on a callback. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/f1e9ef6b59e6
Use digests instead of sources to key cached binaries. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/88fa86e78607
Add versioning support for serialized shaders. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/1ccc107e3483
Cache exactly the shaders that are used early in startup. r=mattwoodrow,sotaro
https://hg.mozilla.org/integration/autoland/rev/e52996e78762
Follow-up to fix Btup build bustage. r=bholley
https://hg.mozilla.org/integration/autoland/rev/d244292c2a12
Make tup build typenum. r=frodynj
(For those following along I filed bug 1511648 to fix the tup thing better).
Depends on: 1511726
Noticed some big perf improvements on Windows 10 QR:

== Change summary for alert #17985 (as of Sat, 01 Dec 2018 16:47:54 GMT) ==

Improvements:

 20%  Heap Unclassified opt stylo     71,875,763.71 -> 57,683,994.65
  4%  Explicit Memory opt stylo       367,216,126.73 -> 352,745,281.92

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=17985
You need to log in before you can comment on or make changes to this bug.