Skip to content

Minutes 2021 03 08

Corentin Wallez edited this page Mar 11, 2021 · 1 revision

GPU Web 2021-03-08

Chair: Corentin

Scribe: Austin / Kai / Ken

Location: Google Meet

Tentative agenda

Attendance

  • AMD
    • Chas Boyd
  • Apple
    • Myles C. Maxfield
  • Google
    • Austin Eng
    • Brandon Jones
    • Corentin Wallez
    • Kai Ninomiya
    • Ken Russell
    • Shrek Shao
  • Intel
    • Yunchao He
  • Microsoft
    • Damyan Pepper
    • Rafael Cintron
  • Mozilla
    • Dzmitry Malyshau
    • Jeff Gilbert
  • Dominic Cerisano
  • Francois Daoust
  • Francois Guthmann
  • Michael Shannon
  • Timo de Kort

Method of ensuring GPUShaderModules can contain MTLLibraries #1064

  • MM: Looks like direction of the conversation is a new function which warms up pipelines. You would give it at least one shader module, and a bunch of stuff that would go in the pipeline descriptor. Each one of those items would be optional. They can call it with whatever information they happen to know, and the browser might do something with that.
  • CW: That matches my understanding. Think we’re making a local maximum in this part of the design space.
  • MM: This design would solve Apple’s concern. Fine with us if we go this way. For ergonomics, putting it on the shader module makes sense. GPUShaderModule.warmpup(...).
  • CW: If it’s there, how do you know if it ‘s for vertex/frag/compute.
  • MM: You’d say sahderModule.warmup([{ }, ]). You pass an array, and the items of the array specify the entrypoints it applies to.
  • CW: something we realized recently: in D3D12, there are strong rules about varyings matching between vertex/fragment shaders. This group needs to talk about it. Also if we need to do shader instrumentation between these shaders, might be more complicated.
  • DM: we made full circle here. At F2F we were thinking about hint during ShaderModule creation. Then back to create pipelines, multiple pipelines. Then said, fields optional. Then, different entry point. Now we’re back to a hint. Same as separate entry point that warms up ShaderModule.
  • JG: did we end up losing where we attach ShaderModules to pipelines? I don’t see it in the spec.
  • CW: in pipelines you have stages, vertex/fragment.
  • JG: i forgot how deeply this is nested.
  • CW: Think createWarmupPipelines seems safer..?
  • MM: reason we ended up on this proposal: as Brandon described, he knows some state at different times than others. Would like to run some kind of compilation before all the state is known.
  • JG: example of state you don’t know?
  • MM: Doesn’t know the vertex descriptor.
  • BJ: IN this case, I might be loading a glTF file. For all the files, they’re all going to share basically the same material description. So I’ll probably know all the pipeline layouts. However, there might be wild variability between the different vertex buffer layouts. I would evaluate that model-by-model. I would gladly provide some information, but not all of it. Not clear if providing the information would help or not. And it’s not clear if I have to “fake” some of the information (vertex buffers) to provide the pipeline layout.
  • MM: that’s why I like this optional proposal. You don’t have to fake anything. You give the browser what you know. If you don’t know enough the browser ignores it. If you do then you get the perf boost.
  • CW: For Chromium on Metal, because of how we do vertex fetch, we wouldn’t be able to precompile without the vertex state, BUT we would be able to precompile the fragment shader, which is often the heavier one.
  • MM: makes sense.
  • CW: is it time where we say this is the direction we want to go? Someone write the spec?
  • DM: do people have strong feelings about this being part of ShaderModuleDescriptor vs. separate entry point?
  • MM: should be on ShaderModule rather than Descriptor.
  • DM: doesn’t have to be separate function at all. Hints could be carried in ShaderModuleDescriptor.
  • MM: so createShaderModule, you’d give it a string and bunch of extra stuff?
  • DM: yes, hint structures.
  • MM: ok with me.
  • JG: distinction is that if you call warmup() twice with different stuff, unclear what we should do. But if you createShaderModule with hints it’s more clear.
  • MM: I think if we go this approach, then architecturally, the spec would say warmup() does nothing, so perhaps the problem you described is not a problem.
  • JG: we could punt it from the spec level, but that’s not good for the ecosystem level. What should people expect it to do? Not guaranteeing it does anything, but it’s not responsible for us to say “it does nothing, use it if you feel like it”.
  • MM: I was envisioning that the spec would say that it does nothing, but browser’s “should” do any kind of precompilation possible at the time the function is called. Now it could happen asynchronously.
  • JG: my concern - observable characteristics. In one browser warmup replaces previously warmed-up shader module, another one supplements it - works great in one browser, not in another. Simpler to put it in createShaderModule.
  • KN: putting it in createShaderModule breaks this being a strict improvement. Apps have to choose what to put in createShaderModule to e.g. avoid parsing multiple times, vs. warmup working less well. I think we should do warmup().
  • JG: what happens if I call warmup() multiple times?
  • KN: It warms up a cache. I don’t think the application should expect it to replace anything.
  • MM: another way to solve: if function takes an array, calling it twice is as-if you concatenated the arrays and called it once.
  • CW: Warmup is priming a cache, and the spec will not well-specify how the cache works. Generally it means that caches aggregate stuff.
  • JG: my concern - we’re adding this to spec because we think it’s important to give users better experience, but we’re abdicating our responsibility in not telling users exactly how to take advantage of it.
  • CW: I think we should still do it pipeline-based for multiple reasons. It’s much more clear - you can put stuff from multiple shader modules. If we want we can even merge modules together in a batched create pipeline. And lastly, it makes it more clear which shader module goes where. With shaderModule.warmup(..) you need to specify the entrypoints, etc.
  • MM: and this would be handled by a flag? What do you mean pipeline-based.
  • CW: Either a flag or two separate lists of pipeline descriptors.
  • JG: is this an alternate proposal?
  • CW: it’s what’s in the bug.
  • MM: I’m convinced by your argument that people will want to mix and match ShaderModules. Putting .warmup() on single shader module fails if you want to use vert from one module and frag from another. Don’t think it should be part of createShaderModule create*PipelinesAsync because 1) these descriptors won’t return any values 2) all the items in these descriptors are optional. Difference in both input and output of this function. Should be separate call rather than overloading an existing one.
  • CW: you meant create*PipelinesAsync?
  • MM: yes.
  • DM: think I disagree. You’re saying people will mix/match shader modules. This is correct. But the task here is to align our shader modules to MtlShaderModules. Not to make it so MtlShaderModules encompass multiple GPUShaderModules. If people write things together in a shader module they should be together in the MtlShaderModule, but we shouldn’t have to merge multiple ones into a single MtlShaderModule.
  • MM: that’s fair.
  • CW: that’s just an opportunity for optimization. Gist of it is - you want to tell the browser “I want to compile all these things, you figure out how to do it.”
  • MM: another argument - say the author calls this function with every single piece of a PipelineDescriptor. Browser would want to create a real pipeline. Why wouldn’t it want to? If we put it on ShaderModule, less likely the browser could do this.
  • DM: why would browsers create pipelines on warmup? Don’t think that’s the goal.
  • MM: not a goal, just a happy opportunity.
  • DM: don’t think we’ll create pipelines. Author doesn’t give us enough information. And we can do it later.
  • JG: there are totally shaders we can precompile pipelines for, like blit shaders.
  • MM: think those folks would call createPipeline early.
  • CW: don’t want to block this too much. Reasoning - close to what Metal’s doing - there’s a compiler service somewhere in the browser. I just want to give it everything at once, and it can tell me when it’s finished. Sending a big batch request. Then you know you’re on the fastest path you can be. Maybe it’s too impractical?
  • MM: DM do you prefer putting it on ShaderModule?
  • DM: warmup where you specify 1 PipelineLayout and bunch of hint structures would work best for me. Can still call multiple times for multiple layouts, but that’s the structure best for me.
  • MM: why are Pipeline Layouts special?
  • DM: you rarely change them within the module. When I write shaders I think, this set of entry points would work with this PipelineLayout. Almost never use same set of entry points with different PipelineLayouts. Also encodes state you need to generate Metal ArgumentBuffers.
  • MM: fine with me.
  • CW: I think we’d need to discuss more internally what this would look like.
  • MM: OK. Should we take this up next week?
  • CW: ok, yes, let’s do that.
  • MM: thanks for devoting so much time over the last few calls to this topic. Our team appreciates it.

Zero initializing textures can not be done efficiently #1422

  • DM: We found a problem with the current specification, which requires us to zero out textures. Thought we would just replace Load with Clear. But now implementing it, we can’t, because we encode the render pass in realtime as the app issues encoding commands (not delayed like Dawn). But we can’t choose a loadOp until submit. E.g. on Metal we would start a render pass just to clear, end it, then start the user’s render pass. 3x bandwidth just so we can do the lazy clear. Goes against what we thought - that it would be the same cost as just tracking it. Hence my wish to go back to this problem and see if we can find a better solution.
  • CW: Only time it happens after app initialization is if the app does storeOp=”clear” to discard, and then loads from it again. That’s also a bit of an application bug though.
  • DM: (example about game with decal.) In chrome might always work but in firefox hitch every 60 frames.
  • CW: something i suggested we could do - understand perf difference between browsers. Could do - add performance warning for this, even if not applicable to us right now. Could also deoptimize the code. Doesn’t matter that much. If we wanted same perf everywhere and to put a warning, probably fine.
  • KN: question about Dawn. Thought Dawn encoded command buffer on finish, not on submit.
  • CW: on submit.
  • KN: ok. Even if you defer encoding have to do so until submit?
  • CW: yes.
  • DM: one motivating factor for us - hard to track which part of textures are initialized & not. Implementation detail. If you copy into everything but 1 texel, don’t want to make validation error. Seems to still be valuable. If Chromium adds this warning we should be ok.
  • CW: We can definitely do that. If that concern is enough to add state [to the spec] etc, it would be much better for Chromium to just add the warning.
  • MM: can one of you describe the situation where lazy clearing would occur?
  • CW: Sure. You create a texture, say albedo for your model. You don’t want to zero it on creation, because you might avoid it if the developer does full copies into the mip levels. Since each mip level is completely written in a copy, it’s easy to track and clear only when needed, to take advantage of the developer doing things correctly. The other case, which we would add a warning for: When you use loadOp Load, but the texture is uninitialized, we replace it with loadOp Clear. Which is only possible if you make the same implementation choice as us [i.e. not the one wgpu makes].
  • KR: This has come up in ANGLE a couple of months ago. There was a situation where copyTexSubImage2D into a just-allocated destination texture. It was too hard to track the subrectangles, so we just did a full clear and then the copy. Hard with 3D textures to track volumes, etc.
  • JG: Firefox actually does the 3d texture optimizations. Not the subrect, but the layers.
  • CW: In Dawn we’re doing per-subresource tracking - for each subresource, does this have any uninit data.
  • MM: So if you copy from an uninitialized thing to something else, then the uninitialized bit is viral…
  • JG: Usually just clear before doing the copy.
  • MM: This lazy clearing only happens for newly created resources?
  • DM: Also for those that are discarded in the render pass. Would affect depth textures and multisample textures, most likely.
  • CW: Do we need to change storeOp “clear” to “discard”? We’re going to be telling the developer (via warnings) that they have to storeOp Clear then loadOp Clear. Looks like duplicated work but isn’t.
  • DM: I think that’s a good idea.
  • CW: Okay, we’ll add a warning in Chromium. Are we satisfied with lazy clear? Then offline we can discuss about making the names more clear.
  • DM: acceptable.
  • MM: concerned about Chrome adding warning because “other browsers” have performance characteristics. Not intuitive this would actually work as a long-term solution.
  • DM: Agree but I don’t have a better idea.
  • KN: think it’s a better solution than changing the validation to make this an error.
  • JG: agree.

Require [SecureContext] for using WebGPU. #1363

  • CW: Bunch of discussions by various folks not in this call. Do we feel like we can make progress on this in this meeting?
  • (silence)
  • DM: who do we need? We have Myles.
  • CW: Myles quoted Maciej, asked Chris Thompson to discuss from Chromium Security’s standpoint. Anne is from Mozilla.
  • RC: do all external people agree?
  • CW: no.
  • CW: one thing they agree on: would be nice if TAG had concrete guidelines on what needs [SecureContext] and not.
  • JG: would be nice if that were the case. Lacking that, should probably write both sides of this, send to TAG, say “give us a ruling”. Best to ask for clarification.
  • CW: or - what about writing another couple explainers, then filing for TAG review? Please?
  • JG: what do we need to do?
  • CW: bunch of them. Kai and I took 2 each. One should come as PR soon.
  • CW: don’t need all of them to file TAG review - but would be nice to have at least half.
  • JG: orthogonal to first public draft?
  • CW: yes, orthogonal.
  • CW: if someone feels confident enough to ask TAG about this particular conversation - please do, OK to do so.
  • MM: quick question. Post Chris wrote 12 days ago with bullet points. “As a security reviewer for this API, I think…” does he speak for himself or the Chrome team?
  • CW: he speaks as security reviewer for the Chrome team. People internally were also saying it’d be good to have clear guidelines from the TAG. Any set of guidelines that comes from the TAG should have WebGPU in list of things that should have SecureContext. That’s a belief. Not something you can put as Chromium team’s thing. Written policy that’s linked. That’s the closest thing to Chrome’s voice on this.

Agenda for next meeting

  • More on ShaderModule hints
  • Other topics?
  • Can build this offline. Feel free to edit this directly, or add to Needs-Discussion column of WebGPU project.
  • KN: we’ll definitely have stuff but don’t have time this week to fill out agenda. Will see if we can fill out agenda in Editors’ meeting.
Clone this wiki locally