webnn: Remove the MLContext.compute() method [chromium/src : main]
ningxin hu (Gerrit)
ningxin hu added 1 comment![]( "Open in Gerrit")
@asully What's the plan for this CL?
The webnn-samples and WebNN EP haven used dispatch. WebNN dev preview demos are moving to use latest ORT WebNN EP, so that will use dispatch soon. I think it is ready for deprecate compute. And @bryan.b...@intel.com also has some optimization work for dispatch pending on removing compute path.
https://github.com/webmachinelearning/webnn-samples/pull/276
https://github.com/microsoft/onnxruntime/pull/21301
Related details
- Austin Sullivan
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Austin Sullivan (Gerrit)
Austin Sullivan added 1 comment![]( "Open in Gerrit")
@asully What's the plan for this CL?
The webnn-samples and WebNN EP haven used dispatch. WebNN dev preview demos are moving to use latest ORT WebNN EP, so that will use dispatch soon. I think it is ready for deprecate compute. And @bryan.b...@intel.com also has some optimization work for dispatch pending on removing compute path.
https://github.com/webmachinelearning/webnn-samples/pull/276
https://github.com/microsoft/onnxruntime/pull/21301
I'd like to merge it as soon as possible!
Unfortunately this CL makes ~all the WPTs fail, since they overwhelmingly use `compute()`. So this CL is blocked on https://crrev.com/c/5556220. @feng...@intel.com and I discussed this here: https://chromium-review.googlesource.com/c/chromium/src/+/5556220/comment/dab5ab45_a091cf23/
Related details
- ningxin hu
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Feng Dai (Gerrit)
Feng Dai added 1 comment![]( "Open in Gerrit")
Austin Sullivan@asully What's the plan for this CL?
The webnn-samples and WebNN EP haven used dispatch. WebNN dev preview demos are moving to use latest ORT WebNN EP, so that will use dispatch soon. I think it is ready for deprecate compute. And @bryan.b...@intel.com also has some optimization work for dispatch pending on removing compute path.
https://github.com/webmachinelearning/webnn-samples/pull/276
https://github.com/microsoft/onnxruntime/pull/21301
I'd like to merge it as soon as possible!
Unfortunately this CL makes ~all the WPTs fail, since they overwhelmingly use `compute()`. So this CL is blocked on https://crrev.com/c/5556220. @feng...@intel.com and I discussed this here: https://chromium-review.googlesource.com/c/chromium/src/+/5556220/comment/dab5ab45_a091cf23/
@asu...@chromium.org My CL https://chromium-review.googlesource.com/c/chromium/src/+/5556220 has been landed, please rebase this CL, thanks.
Related details
- Austin Sullivan
- ningxin hu
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Austin Sullivan (Gerrit)
Austin Sullivan added 2 comments![]( "Open in Gerrit")
Hey @phi...@chromium.org PTAL at this overall change? It passes tests locally so I'm speculatively putting it up for review 🤞
Once it LGTY I'll add mojom and VirtualTestSuite owners
Note: I turned on an IWYU tool to make sure I removed #includes which are now unused with `compute()` gone, hence all these #include changes
Related details
- Phillis Tang
- ningxin hu
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Phillis Tang (Gerrit)
Phillis Tang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Phillis Tang
| Code-Review | +1 |
1 comment
LGTM! we will need to rebaseline the idlharness expectation after the spec is updated?
Related details
- Austin Sullivan
- ningxin hu
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Austin Sullivan (Gerrit)
Austin Sullivan added 2 comments![]( "Open in Gerrit")
LGTM! we will need to rebaseline the idlharness expectation after the spec is updated?
Correct. I figure this CL will likely merge before the spec changes, so this seems fine for now
PTAL? Thanks!
- @aj...@chromium.org webnn_graph.mojom
- @weiz...@google.com VirtualTestSuites
Related details
- Alex Gough
- Weizhong Xia
- ningxin hu
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Gough (Gerrit)
Alex Gough voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alex Gough
| Code-Review | +1 |
1 comment
- Austin Sullivan
- Weizhong Xia
- ningxin hu
Austin Sullivan (Gerrit)
Austin Sullivan voted Auto-Submit+1![]( "Open in Gerrit")
- Weizhong Xia
- ningxin hu
ningxin hu (Gerrit)
ningxin hu added 4 comments![]( "Open in Gerrit")
std::unique_ptr<ComputeResources> compute_resources_;This member is not needed anymore.
RecordGraphExecutionOnBackgroundThread(Same here, this method is not needed anymore.
static HRESULT RecordGraphExecution(This method can be removed as well.
struct ComputeResources {I suppose the `ComputeResources` can be removed.
Related details
- Austin Sullivan
- Weizhong Xia
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Austin Sullivan (Gerrit)
Austin Sullivan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Austin Sullivan
| Auto-Submit | +1 |
1 comment
RecordGraphExecutionOnBackgroundThread(Same here, this method is not needed anymore.
Hmm `RecordGraphExecutionOnBackgroundThread` is still called during `OnInitializationComplete`. Are you sure we can remove it?
Related details
- Weizhong Xia
- ningxin hu
Austin Sullivan (Gerrit)
Austin Sullivan removed a vote from this change![]( "Open in Gerrit")
Removed Auto-Submit+1 by Austin Sullivan <asu...@chromium.org>
Related details
- Weizhong Xia
- ningxin hu
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
ningxin hu (Gerrit)
ningxin hu added 1 comment![]( "Open in Gerrit")
RecordGraphExecutionOnBackgroundThread(Austin SullivanSame here, this method is not needed anymore.
Hmm `RecordGraphExecutionOnBackgroundThread` is still called during `OnInitializationComplete`. Are you sure we can remove it?
It is only used to pre-record the command list for compute (in compute resources). I can open an issue and remove them in a follow-up CL, if you are fine with that. WDYT?
Related details
- Austin Sullivan
- Weizhong Xia
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Austin Sullivan (Gerrit)
Austin Sullivan added 1 comment![]( "Open in Gerrit")
RecordGraphExecutionOnBackgroundThread(Austin SullivanSame here, this method is not needed anymore.
ningxin huHmm `RecordGraphExecutionOnBackgroundThread` is still called during `OnInitializationComplete`. Are you sure we can remove it?
It is only used to pre-record the command list for compute (in compute resources). I can open an issue and remove them in a follow-up CL, if you are fine with that. WDYT?
- Weizhong Xia
- ningxin hu
Weizhong Xia (Gerrit)
Weizhong Xia voted Code-Review+1![]( "Open in Gerrit")
- Austin Sullivan
- ningxin hu
ningxin hu (Gerrit)
ningxin hu voted and added 5 comments![]( "Open in Gerrit")
Votes added by ningxin hu
| Code-Review | +1 |
5 comments
std::unique_ptr<ComputeResources> compute_resources_;This member is not needed anymore.
Tracked by https://issues.chromium.org/issues/378779891
RecordGraphExecutionOnBackgroundThread(Austin SullivanSame here, this method is not needed anymore.
ningxin huHmm `RecordGraphExecutionOnBackgroundThread` is still called during `OnInitializationComplete`. Are you sure we can remove it?
Austin SullivanIt is only used to pre-record the command list for compute (in compute resources). I can open an issue and remove them in a follow-up CL, if you are fine with that. WDYT?
SGTM let's do that :)
Opened https://issues.chromium.org/issues/378779891, I'll upload a CL after this CL landed.
static HRESULT RecordGraphExecution(This method can be removed as well.
Tracked by https://issues.chromium.org/issues/378779891
struct ComputeResources {I suppose the `ComputeResources` can be removed.
Tracked by https://issues.chromium.org/issues/378779891
Related details
- Austin Sullivan
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Austin Sullivan (Gerrit)
Austin Sullivan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Austin Sullivan
| Commit-Queue | +2 |
1 comment
Thanks for all the reviews! Submitting now 🎉
Related details
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
webnn: Remove the MLContext.compute() method
This method has been deprecated for dispatch()
OBSOLETE_HISTOGRAMS=compute() is deprecated in favor of dispatch()
- M services/webnn/coreml/graph_impl_coreml.h
- M services/webnn/coreml/graph_impl_coreml.mm
- M services/webnn/dml/context_impl_dml.h
- M services/webnn/dml/graph_impl_dml.cc
- M services/webnn/dml/graph_impl_dml.h
- M services/webnn/error.h
- M services/webnn/public/mojom/webnn_graph.mojom
- M services/webnn/tflite/graph_impl_tflite.cc
- M services/webnn/tflite/graph_impl_tflite.h
- M services/webnn/webnn_context_impl.cc
- M services/webnn/webnn_context_impl.h
- M services/webnn/webnn_context_provider_impl.cc
- M services/webnn/webnn_graph_builder_impl_unittest.cc
- M services/webnn/webnn_graph_impl.cc
- M services/webnn/webnn_graph_impl.h
- M services/webnn/webnn_graph_impl_backend_test.cc
- M services/webnn/webnn_graph_impl_unittest.cc
- M third_party/blink/renderer/bindings/generated_in_modules.gni
- M third_party/blink/renderer/modules/ml/ml_context.cc
- M third_party/blink/renderer/modules/ml/ml_context.h
- M third_party/blink/renderer/modules/ml/ml_context.idl
- M third_party/blink/renderer/modules/ml/webnn/ml_graph.cc
- M third_party/blink/renderer/modules/ml/webnn/ml_graph.h
- M third_party/blink/renderer/modules/ml/webnn/ml_graph_test.cc
- M third_party/blink/renderer/modules/ml/webnn/ml_graph_utils.cc
- M third_party/blink/renderer/modules/ml/webnn/ml_graph_utils.h
- M third_party/blink/web_tests/NeverFixTests
- M third_party/blink/web_tests/VirtualTestSuites
- D third_party/blink/web_tests/external/wpt/webnn/conformance_tests/compute-arraybufferview-with-bigger-arraybuffer.https.any.js
- D third_party/blink/web_tests/external/wpt/webnn/conformance_tests/parallel-compute.https.any.js
- D third_party/blink/web_tests/external/wpt/webnn/validation_tests/compute-multiple-arraybufferviews-sharing-same-arraybuffer.https.any.js
- M third_party/blink/web_tests/external/wpt/webnn/validation_tests/constant-changed-buffer.https.any.js
- M third_party/blink/web_tests/external/wpt/webnn/validation_tests/destroyContext.https.any.js
- M third_party/blink/web_tests/external/wpt/webnn/validation_tests/destroyGraph.https.any.js
- M third_party/blink/web_tests/external/wpt/webnn/validation_tests/pooling-and-reduction-keep-dims.https.any.js
- M third_party/blink/web_tests/virtual/webnn-service-on-cpu/external/wpt/webnn/idlharness.https.any-expected.txt
- M third_party/blink/web_tests/virtual/webnn-service-on-cpu/external/wpt/webnn/idlharness.https.any.worker-expected.txt
- M tools/metrics/histograms/metadata/webnn/histograms.xml
Code-Review: +1 by Alex Gough, +1 by Phillis Tang, +1 by Weizhong Xia, +1 by ningxin hu
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/49135
Related details
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |