Eager shader compilation #5606

kazcw · 2023-02-09T19:52:19Z

Pull Request Description

Start shader compilation eagerly, rather than wait until they are needed. Implements #5604.

Before:

We didn't compile shaders until we needed them, causing latency before new widgets can render.

After:

Shaders are done compiling before project_open finishes.

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

kazcw · 2023-02-09T19:55:04Z

A lot of shaders are missing the cache--putting this back in draft while I look into it.

wdanilo · 2023-02-09T20:54:07Z

This may be connected with one thing - when we move a shape system to a layer which did not have earlier this shape system, we might submit the shader again. I do not think that we are re-using the shaders in this situation. If this is the case, it might be good to improve it in this PR as well.

enso-bot · 2023-02-10T03:16:16Z

Keziah Wesley reports a new STANDUP for today (2023-02-09):

Progress: Started working on eager shader compilation. Implemented support for persistent shaders, and compiling unpaired shaders. It should be finished by 2023-02-10.

Next Day: Next day I will be working on the #5606 task. Debug shader compile errors.

farmaazon · 2023-02-13T09:59:34Z

I do not think that we are re-using the shaders in this situation.

I can confirm, that indeed we don't do that.

enso-bot · 2023-02-14T04:01:14Z

Keziah Wesley reports a new 🔴 DELAY for the last Friday (2023-02-10):

Summary: There is 4 days delay in implementation of the Eager shader compilation (#5606) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: In initial estimate, I thought we had ready-to-compile shader code. Constructing the input declarations at application start will require refactoring some functionality.

enso-bot · 2023-02-14T04:01:41Z

Keziah Wesley reports a new STANDUP for the last Friday (2023-02-10):

Progress: Using the Shape constructors to complete the shader programs, but constructing the shapes early is having some side effect that breaks rendering. It should be finished by 2023-02-14.

Next Day: Next day I will be working on the #5606 task. Debug why constructing Shapes breaks rendering.

enso-bot · 2023-02-14T23:02:41Z

Keziah Wesley reports a new STANDUP for yesterday (2023-02-13):

Progress: Debugging the render problem when shape constructors are run. It should be finished by 2023-02-14.

Next Day: Next day I will be working on the #5606 task. Finish the PR.

enso-bot · 2023-02-14T23:03:56Z

Keziah Wesley reports a new STANDUP for today (2023-02-14):

Progress: Finished eager shader compilation working. It should be finished by 2023-02-14.

Next Day: Next day I will be working on the #5603 task. Profile and look for frontend optimization opportunities.

kazcw · 2023-02-15T15:18:36Z

@wdanilo

This may be connected with one thing - when we move a shape system to a layer which did not have earlier this shape system, we might submit the shader again. I do not think that we are re-using the shaders in this situation. If this is the case, it might be good to improve it in this PR as well.

I think this is handled by the caching layer in the shader compiler driver--the same shader code won't be compiled more than once.

wdanilo

As always, beautiful and well written code ❤️

Few additional questions:

Does this PR enable precompiled vertex shaders?
Is there any callback when the shaders are ready? I want to use it for spinner hiding.

lib/rust/ensogl/core/src/display/shape/primitive/system.rs

lib/rust/ensogl/core/src/display/symbol/gpu/shader.rs

wdanilo · 2023-02-18T23:14:37Z

lib/rust/ensogl/core/src/display/world.rs

+#[profile(Task)]
+pub fn instantiate_shaders() {
+    SHAPES_DEFINITIONS.with_borrow(|shapes| {
+        for shape in shapes {
+            let path = &shape.definition_path;
+            let preload = match path {
+                _ if path.starts_with("app/gui/view/debug_scene/") => false,
+                _ if path.starts_with("lib/rust/ensogl/example/") => false,
+                _ => true,
+            };
+            if preload {
+                let _shape = (shape.cons)();
+            }
+        }
+    });
+}


This looks super hacky to me. What about this:

Renaming app/gui/view/debug_scene/ to app/gui/view/examples/

Renaming lib/rust/ensogl/example/ to lib/rust/ensogl/examples/

Excluding here anything that has folder examples in path.

Writing that in our docs?

That's a bit less hacky! Implemented.

lib/rust/ensogl/core/src/display/world.rs

lib/rust/ensogl/core/src/system/gpu/shader.rs

kazcw · 2023-02-20T19:12:04Z

I don't think QA is applicable to this--it is tested by profiling and looking at the graph, which I have posted in the PR description above; so I'll merge as soon as I pass code owner review.

kazcw · 2023-02-20T20:44:30Z

@wdanilo

1. Does this PR enable precompiled vertex shaders?

I haven't added support for offline vertex shader optimization; however I am eagerly linking the shader programs as well as compiling, so the shaders are ready to use when needed.

2. Is there any callback when the shaders are ready? I want to use it for spinner hiding.

The shader compiler has an on_idle mechanism, but that's very low-level. I'll write an async function in Scene that can be used to initiate compiling all the shaders, and await completion.

wdanilo · 2023-02-21T14:28:07Z

@kazcw can you add also vertex shader recompilation? It looks like it should just work with your changes now. Just test it out, if there would be problems, we will do it in separate PR (in such a case create a task for it pls). If its doable now, let's do it.

kazcw · 2023-02-21T19:40:00Z

@wdanilo

@kazcw can you add also vertex shader recompilation? It looks like it should just work with your changes now. Just test it out, if there would be problems, we will do it in separate PR (in such a case create a task for it pls). If its doable now, let's do it.

It was almost working. The dummy input declarations we use for shader extraction needed to be stripped. Now using fully-optimized shaders!

I've also added async completion notification--you can use scene.prepare_to_render().await, which will finish when the shaders are ready (it is idempotent--if compilation has already been started, it will just wait and return when they're ready).

wdanilo · 2023-02-21T22:27:28Z

This is amazing!

enso-bot · 2023-02-22T06:33:04Z

Keziah Wesley reports a new 🔴 DELAY for yesterday (2023-02-20):

Summary: There is 6 days delay in implementation of the Eager shader compilation (#5606) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: Review latency.

enso-bot · 2023-02-22T06:33:11Z

Keziah Wesley reports a new STANDUP for yesterday (2023-02-20):

Progress: Addressed review on shader compilation--added a little more functionality. It should be finished by 2023-02-20.

Next Day: Next day I will be working on the #5669 task. Look in to text rendering.

eager shader compilation

050d51b

kazcw self-assigned this Feb 9, 2023

kazcw requested review from MichaelMauderer, wdanilo and farmaazon as code owners February 9, 2023 19:52

kazcw marked this pull request as draft February 9, 2023 19:54

kazcw added the CI: No changelog needed Do not require a changelog entry for this PR. label Feb 9, 2023

support compiling unpaired shaders

04b0164

kazcw added 3 commits February 10, 2023 18:17

clean up

9cb79bc

Merge remote-tracking branch 'origin/develop' into eager-shaders

f9e271d

fix

3f364a3

kazcw added 8 commits February 14, 2023 20:27

fix log-level changing bug

6648441

cleanup

a20e168

cleanup

40c674d

cleanup

b56fcb0

set up shader-program inputs correctly

cbfb375

program inputs

177f105

Merge remote-tracking branch 'origin/develop' into eager-shaders

e8d3946

fix & cleanup

4332f68

kazcw marked this pull request as ready for review February 14, 2023 23:01

kazcw requested a review from mwu-tow as a code owner February 14, 2023 23:01

kazcw added the CI: Keep up to date Automatically update this PR to the latest develop. label Feb 17, 2023

kazcw and others added 3 commits February 17, 2023 10:18

Merge branch 'develop' into wip/kw/eager-shaders

73f485c

Merge branch 'develop' into wip/kw/eager-shaders

4d21fcf

Merge branch 'develop' into wip/kw/eager-shaders

4ee6d15

wdanilo approved these changes Feb 18, 2023

View reviewed changes

mergify bot and others added 7 commits February 19, 2023 00:38

Merge branch 'develop' into wip/kw/eager-shaders

703ba4f

Merge branch 'develop' into wip/kw/eager-shaders

8121adb

Merge branch 'develop' into wip/kw/eager-shaders

069d868

Merge branch 'develop' into wip/kw/eager-shaders

c3c7ffe

Apply review.

efcd01d

Apply review.

da7bb1b

Document crate path magic.

aa43189

kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Feb 20, 2023

Merge branch 'develop' into wip/kw/eager-shaders

faa8fd8

mergify bot added 3 commits February 20, 2023 21:25

Merge branch 'develop' into wip/kw/eager-shaders

898a2d1

Merge branch 'develop' into wip/kw/eager-shaders

00447dc

Merge branch 'develop' into wip/kw/eager-shaders

3dd5ba8

kazcw added 2 commits February 21, 2023 19:32

Use optimized vertex shaders.

b186a53

Support awaiting shader compilation

681a1a9

wdanilo approved these changes Feb 21, 2023

View reviewed changes

wdanilo merged commit dd3ee76 into develop Feb 21, 2023

wdanilo deleted the wip/kw/eager-shaders branch February 21, 2023 23:29

kazcw linked an issue Feb 23, 2023 that may be closed by this pull request

Pre-emptive shader compilation #5604

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eager shader compilation #5606

Eager shader compilation #5606

kazcw commented Feb 9, 2023 •

edited

Loading

kazcw commented Feb 9, 2023

wdanilo commented Feb 9, 2023

enso-bot bot commented Feb 10, 2023

farmaazon commented Feb 13, 2023

enso-bot bot commented Feb 14, 2023

enso-bot bot commented Feb 14, 2023

enso-bot bot commented Feb 14, 2023

enso-bot bot commented Feb 14, 2023

kazcw commented Feb 15, 2023

wdanilo left a comment

wdanilo Feb 18, 2023

kazcw Feb 20, 2023

kazcw commented Feb 20, 2023

kazcw commented Feb 20, 2023

wdanilo commented Feb 21, 2023

kazcw commented Feb 21, 2023

wdanilo commented Feb 21, 2023

enso-bot bot commented Feb 22, 2023

enso-bot bot commented Feb 22, 2023

Eager shader compilation #5606

Eager shader compilation #5606

Conversation

kazcw commented Feb 9, 2023 • edited Loading

Pull Request Description

Checklist

kazcw commented Feb 9, 2023

wdanilo commented Feb 9, 2023

enso-bot bot commented Feb 10, 2023

farmaazon commented Feb 13, 2023

enso-bot bot commented Feb 14, 2023

enso-bot bot commented Feb 14, 2023

enso-bot bot commented Feb 14, 2023

enso-bot bot commented Feb 14, 2023

kazcw commented Feb 15, 2023

wdanilo left a comment

Choose a reason for hiding this comment

wdanilo Feb 18, 2023

Choose a reason for hiding this comment

kazcw Feb 20, 2023

Choose a reason for hiding this comment

kazcw commented Feb 20, 2023

kazcw commented Feb 20, 2023

wdanilo commented Feb 21, 2023

kazcw commented Feb 21, 2023

wdanilo commented Feb 21, 2023

enso-bot bot commented Feb 22, 2023

enso-bot bot commented Feb 22, 2023

kazcw commented Feb 9, 2023 •

edited

Loading