Bug 200200

Summary: [WebGPU] Replace Vectors with HashSets for tracking resources used by GPUCommandBuffer
Product: WebKit Reporter: Justin Fan <justin_fan>
Component: New BugsAssignee: Justin Fan <justin_fan>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-watchlist, graouts, kondapallykalyan, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Justin Fan
Reported 2019-07-27 17:37:11 PDT
[WebGPU] Replace Vectors with HashSets for tracking resources used by GPUCommandBuffer
Attachments
Patch (3.42 KB, patch)
2019-07-27 17:41 PDT, Justin Fan
no flags
Patch (8.44 KB, patch)
2019-07-27 19:17 PDT, Justin Fan
no flags
Patch (9.27 KB, patch)
2019-07-27 19:28 PDT, Justin Fan
no flags
Patch for landing (8.56 KB, patch)
2019-07-29 12:31 PDT, Justin Fan
no flags
Justin Fan
Comment 1 2019-07-27 17:41:17 PDT
Justin Fan
Comment 2 2019-07-27 19:17:54 PDT
Justin Fan
Comment 3 2019-07-27 19:28:50 PDT
Myles C. Maxfield
Comment 4 2019-07-29 11:10:28 PDT
Comment on attachment 375039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375039&action=review > Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLTypeNamer.cpp:-347 > - auto addResult = m_unnamedTypeMapping.add(&unnamedType, &*iterator); How did this ever compile? I thought unused variables produced errors > Source/WebCore/platform/graphics/gpu/GPUCommandBuffer.h:103 > + void useBuffer(Ref<GPUBuffer>&& buffer) { m_usedBuffers.addVoid(WTFMove(buffer)); } > + void useTexture(Ref<GPUTexture>&& texture) { m_usedTextures.addVoid(WTFMove(texture)); } I see a lot of production, but no consumption. Do the functions that we're using for consumption just happen to be named the same between Vectors and HashSets?
Justin Fan
Comment 5 2019-07-29 12:14:07 PDT
Comment on attachment 375039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375039&action=review >> Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLTypeNamer.cpp:-347 >> - auto addResult = m_unnamedTypeMapping.add(&unnamedType, &*iterator); > > How did this ever compile? I thought unused variables produced errors No idea. Maybe saam has some local flags turned off. EWS doesn't build WebGPU anymore right? >> Source/WebCore/platform/graphics/gpu/GPUCommandBuffer.h:103 >> + void useTexture(Ref<GPUTexture>&& texture) { m_usedTextures.addVoid(WTFMove(texture)); } > > I see a lot of production, but no consumption. Do the functions that we're using for consumption just happen to be named the same between Vectors and HashSets? Command buffers can't be reused in our impl, so I iterate through this container to process them rather than removing from them. They get released when the command buffer is released. Bind groups are immutable once created so no issues there.
Justin Fan
Comment 6 2019-07-29 12:31:34 PDT
Created attachment 375090 [details] Patch for landing
WebKit Commit Bot
Comment 7 2019-07-29 17:06:37 PDT
Comment on attachment 375090 [details] Patch for landing Clearing flags on attachment: 375090 Committed r247930: <https://trac.webkit.org/changeset/247930>
WebKit Commit Bot
Comment 8 2019-07-29 17:06:39 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-07-29 17:07:26 PDT
Note You need to log in before you can comment on or make changes to this bug.