Drop the support of synchronous execution by huningxin · Pull Request #548 · webmachinelearning/webnn

huningxin · 2024-02-01T11:17:11Z

Remove the definition and algorithm steps for

ML.createContextSync()
MLGraphBuilder.buildSync()
MLContext.computeSync()

This PR also removes the synchronous execution description from Programming Model section.

The previous algorithm steps of MLGraphBuilder.build() depends on MLGraphBuilder.buildSync(). The algorithm steps of MLGraphBuilder.buildSync() have issue tracked by #457. That is left to a separate PR. This PR just merges the algorithm steps of MLGraphBuilder.buildSync() into MLGraphBuilder.build() with some modifications:

Remove "The permissions and context validity have been checked..." note regarding to Drop "The permissions and context validity have been checked..." notes #501.
Change throwing exception to rejecting the promise with exception.

Fix #531

@wchao1115 @anssiko @inexorabletash , PTAL. Thanks!

Preview | Diff

inexorabletash · 2024-02-01T18:37:22Z

The bots seem to be having issues, but I've locally verified that there are no Bikeshed warnings or errors.

index.bs

zolkis · 2024-02-02T15:09:58Z

PR preview failed, just wanted to call attention to common usage of "execute graph" and other algorithms from the "Synchronous execution" section also in asynchronous execution. These should be moved or merged into the async execution steps. It's hard to see from the diff if that was the case.

index.bs

anssiko

Thanks @huningxin for preparing this PR swiftly to allow this change get into the next spec milestone.

And thanks @inexorabletash for your careful review.

@wchao1115 do you want to signal your +1 for this synchronous execution removal?

See webmachinelearning/webnn#548 Deletes all sync methods and simplifies the names of all "Async" methods - e.g. BuildAsync() -> Build() Bug: 40283536, 41481333 Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108

Remove the definition and algorithm steps for - ML.createContextSync() - MLGraphBuilder.buildSync() - MLContext.computeSync() Fix webmachinelearning#531

anssiko · 2024-02-15T10:55:50Z

By merging, we're checking some boxes:

This removal gets into the next Candidate Rec Snapshot (CRS) as anticipated in the high-level status https://www.w3.org/TR/webnn/#sotd
This also addresses feedback from earlier TAG review Delta review (to CR) of Web Neural Network API w3ctag/design-reviews#771 (comment) that suggested to drop this feature. (I gave TAG an update on this removal in our delta review.)

SHA: 79baee9 Reason: push, by anssiko Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

See webmachinelearning/webnn#548 Deletes all sync methods and simplifies the names of all "Async" methods - e.g. BuildAsync() -> Build() Bug: 40283536, 41481333 Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108

See webmachinelearning/webnn#548 Deletes all sync methods and simplifies the names of all "Async" methods - e.g. BuildAsync() -> Build() Bug: 40283536, 41481333 Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 Commit-Queue: Austin Sullivan <[email protected]> Reviewed-by: Reilly Grant <[email protected]> Reviewed-by: ningxin hu <[email protected]> Reviewed-by: Alex Gough <[email protected]> Cr-Commit-Position: refs/heads/main@{#1261223}

This reverts commit a3e84dd. Reason for revert: One Win 11 , blink_wpt_tests are failing consistently since https://ci.chromium.org/ui/p/chromium/builders/ci/Win11%20Tests%20x64/19453/blamelist This CL seems to be a candidate for causing this. Original change's description: > webnn: Remove sync methods > > See webmachinelearning/webnn#548 > > Deletes all sync methods and simplifies the names of all "Async" > methods - e.g. BuildAsync() -> Build() > > Bug: 40283536, 41481333 > Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 > Commit-Queue: Austin Sullivan <[email protected]> > Reviewed-by: Reilly Grant <[email protected]> > Reviewed-by: ningxin hu <[email protected]> > Reviewed-by: Alex Gough <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1261223} Bug: 40283536, 41481333 Change-Id: Ib59afe686ae8e18bcad63ff7d15816cd93090f00 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5301381 Owners-Override: Markus Heintz <[email protected]> Auto-Submit: Markus Heintz <[email protected]> Commit-Queue: Markus Heintz <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Cr-Commit-Position: refs/heads/main@{#1261576}

This is a reland of commit a3e84dde09e099bebbe4793ffaa70679168130d6 Original change's description: > webnn: Remove sync methods > > See webmachinelearning/webnn#548 > > Deletes all sync methods and simplifies the names of all "Async" > methods - e.g. BuildAsync() -> Build() > > Bug: 40283536, 41481333 > Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 > Commit-Queue: Austin Sullivan <[email protected]> > Reviewed-by: Reilly Grant <[email protected]> > Reviewed-by: ningxin hu <[email protected]> > Reviewed-by: Alex Gough <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1261223} Bug: 40283536, 41481333 Change-Id: Iffcb922b8175a7181deba1f4c2faa953010ae5c0

This is a reland of commit a3e84dde09e099bebbe4793ffaa70679168130d6 This CL contains a few additional changes: - Enables the async APIs on workers, which was causing bots to fail since workers could not call createContext() - Rebaselines WPTs accordingly - Updates the WPT expectations for the webnn-service-enabled virtual test suite Original change's description: > webnn: Remove sync methods > > See webmachinelearning/webnn#548 > > Deletes all sync methods and simplifies the names of all "Async" > methods - e.g. BuildAsync() -> Build() > > Bug: 40283536, 41481333 > Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 > Commit-Queue: Austin Sullivan <[email protected]> > Reviewed-by: Reilly Grant <[email protected]> > Reviewed-by: ningxin hu <[email protected]> > Reviewed-by: Alex Gough <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1261223} Bug: 40283536, 41481333, 41485898 Change-Id: Iffcb922b8175a7181deba1f4c2faa953010ae5c0

This is a reland of commit a3e84dde09e099bebbe4793ffaa70679168130d6 This CL contains a few additional changes: - Enables the async APIs on workers, which was causing bots to fail since workers could not call createContext() - Rebaselines WPTs accordingly - Updates the WPT expectations for the webnn-service-enabled virtual test suite Original change's description: > webnn: Remove sync methods > > See webmachinelearning/webnn#548 > > Deletes all sync methods and simplifies the names of all "Async" > methods - e.g. BuildAsync() -> Build() > > Bug: 40283536, 41481333 > Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 > Commit-Queue: Austin Sullivan <[email protected]> > Reviewed-by: Reilly Grant <[email protected]> > Reviewed-by: ningxin hu <[email protected]> > Reviewed-by: Alex Gough <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1261223} Cq-Include-Trybots: luci.chromium.try:win11-x64-fyi-rel Bug: 40283536, 41481333, 41485898 Change-Id: Iffcb922b8175a7181deba1f4c2faa953010ae5c0

This is a reland of commit a3e84dde09e099bebbe4793ffaa70679168130d6 This CL contains a few additional changes: - Enables the async APIs on workers, which was causing bots to fail since workers could not call createContext() - Rebaselines WPTs accordingly - Updates the WPT expectations for the webnn-service-enabled virtual test suite Original change's description: > webnn: Remove sync methods > > See webmachinelearning/webnn#548 > > Deletes all sync methods and simplifies the names of all "Async" > methods - e.g. BuildAsync() -> Build() > > Bug: 40283536, 41481333 > Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 > Commit-Queue: Austin Sullivan <[email protected]> > Reviewed-by: Reilly Grant <[email protected]> > Reviewed-by: ningxin hu <[email protected]> > Reviewed-by: Alex Gough <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1261223} Cq-Include-Trybots: luci.chromium.try:win11-x64-fyi-rel Bug: 40283536, 41481333, 41485898 Change-Id: Iffcb922b8175a7181deba1f4c2faa953010ae5c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5303774 Reviewed-by: Reilly Grant <[email protected]> Commit-Queue: Austin Sullivan <[email protected]> Reviewed-by: Rafael Cintron <[email protected]> Reviewed-by: Alex Gough <[email protected]> Cr-Commit-Position: refs/heads/main@{#1262043}

This is a reland of commit a3e84dde09e099bebbe4793ffaa70679168130d6 This CL contains a few additional changes: - Enables the async APIs on workers, which was causing bots to fail since workers could not call createContext() - Rebaselines WPTs accordingly - Updates the WPT expectations for the webnn-service-enabled virtual test suite Original change's description: > webnn: Remove sync methods > > See webmachinelearning/webnn#548 > > Deletes all sync methods and simplifies the names of all "Async" > methods - e.g. BuildAsync() -> Build() > > Bug: 40283536, 41481333 > Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 > Commit-Queue: Austin Sullivan <[email protected]> > Reviewed-by: Reilly Grant <[email protected]> > Reviewed-by: ningxin hu <[email protected]> > Reviewed-by: Alex Gough <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1261223} Cq-Include-Trybots: luci.chromium.try:win11-x64-fyi-rel Bug: 40283536, 41481333, 41485898 Change-Id: Iffcb922b8175a7181deba1f4c2faa953010ae5c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5303774 Reviewed-by: Reilly Grant <[email protected]> Commit-Queue: Austin Sullivan <[email protected]> Reviewed-by: Rafael Cintron <[email protected]> Reviewed-by: Alex Gough <[email protected]> Cr-Commit-Position: refs/heads/main@{#1262043} Co-authored-by: Austin Sullivan <[email protected]>

…estonly Automatic update from web-platform-tests Reland "webnn: Remove sync methods" (#44626) This is a reland of commit a3e84dde09e099bebbe4793ffaa70679168130d6 This CL contains a few additional changes: - Enables the async APIs on workers, which was causing bots to fail since workers could not call createContext() - Rebaselines WPTs accordingly - Updates the WPT expectations for the webnn-service-enabled virtual test suite Original change's description: > webnn: Remove sync methods > > See webmachinelearning/webnn#548 > > Deletes all sync methods and simplifies the names of all "Async" > methods - e.g. BuildAsync() -> Build() > > Bug: 40283536, 41481333 > Change-Id: I3b38d987cb4641ea41ab9a974c46dcba16d5c108 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5292884 > Commit-Queue: Austin Sullivan <[email protected]> > Reviewed-by: Reilly Grant <[email protected]> > Reviewed-by: ningxin hu <[email protected]> > Reviewed-by: Alex Gough <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1261223} Cq-Include-Trybots: luci.chromium.try:win11-x64-fyi-rel Bug: 40283536, 41481333, 41485898 Change-Id: Iffcb922b8175a7181deba1f4c2faa953010ae5c0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5303774 Reviewed-by: Reilly Grant <[email protected]> Commit-Queue: Austin Sullivan <[email protected]> Reviewed-by: Rafael Cintron <[email protected]> Reviewed-by: Alex Gough <[email protected]> Cr-Commit-Position: refs/heads/main@{#1262043} Co-authored-by: Austin Sullivan <[email protected]> -- wpt-commits: 0e308a624676bfea808af80e9a6378ec94dce931 wpt-pr: 44626

inexorabletash suggested changes Feb 1, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

inexorabletash suggested changes Feb 2, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

huningxin requested a review from wchao1115 February 3, 2024 07:08

anssiko approved these changes Feb 5, 2024

View reviewed changes

inexorabletash approved these changes Feb 5, 2024

View reviewed changes

wchao1115 approved these changes Feb 8, 2024

View reviewed changes

chromium-wpt-export-bot mentioned this pull request Feb 14, 2024

webnn: Remove sync methods web-platform-tests/wpt#44572

Closed

huningxin added 3 commits February 15, 2024 16:50

Drop the support of synchronous execution

1aa0bbc

Remove the definition and algorithm steps for - ML.createContextSync() - MLGraphBuilder.buildSync() - MLContext.computeSync() Fix webmachinelearning#531

Use [=reject=] |promise| with a {{TypeError}}

d018fb2

Abort after rejecting promise in parallel steps

7fb3187

huningxin force-pushed the drop_sync branch from bfda418 to 7fb3187 Compare February 15, 2024 08:51

anssiko merged commit 79baee9 into webmachinelearning:main Feb 15, 2024

github-actions bot added a commit that referenced this pull request Feb 15, 2024

Drop the support of synchronous execution (#548)

37c95c0

SHA: 79baee9 Reason: push, by anssiko Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

chromium-wpt-export-bot mentioned this pull request Feb 16, 2024

Reland "webnn: Remove sync methods" web-platform-tests/wpt#44626

Merged

huningxin mentioned this pull request Mar 27, 2024

Revise graph resource validation #622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Drop the support of synchronous execution#548

Drop the support of synchronous execution#548
anssiko merged 3 commits intowebmachinelearning:mainfrom
huningxin:drop_sync

huningxin commented Feb 1, 2024 •

edited by pr-preview bot

Loading

Uh oh!

inexorabletash commented Feb 1, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zolkis commented Feb 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anssiko left a comment

Uh oh!

anssiko commented Feb 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

Conversation

huningxin commented Feb 1, 2024 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

inexorabletash commented Feb 1, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zolkis commented Feb 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anssiko left a comment

Choose a reason for hiding this comment

Uh oh!

anssiko commented Feb 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

huningxin commented Feb 1, 2024 •

edited by pr-preview bot

Loading