Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebLocks]: Modifying weblocks algos to be O(1) #14420

Merged
merged 1 commit into from
Jan 23, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Dec 7, 2018

The behaviour of the request/release operations of web locks are
modified to be O(1) instead of their currently O(n) worst case runtime.
Additionally the query-order wpt is modified to reflect the new spec
requirement that the state returned by navigator.locks.query need only
respect ordering for requested locks per resource.

Bug: 913014
Change-Id: I819f8c27c995cb698a7c8b2c75ee80d32c744f07
Spec: https://wicg.github.io/web-locks/#algorithms
Reviewed-on: https://chromium-review.googlesource.com/c/1367910
Commit-Queue: Andreas Butler <[email protected]>
Reviewed-by: Victor Costan <[email protected]>
Reviewed-by: Joshua Bell <[email protected]>
Reviewed-by: Daniel Murphy <[email protected]>
Cr-Commit-Position: refs/heads/master@{#621833}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1367910 branch 6 times, most recently from 8704a74 to ddf82d8 Compare December 14, 2018 23:57
@jgraham jgraham closed this Dec 17, 2018
@jgraham jgraham reopened this Dec 17, 2018
@foolip foolip force-pushed the chromium-export-cl-1367910 branch from ddf82d8 to 1ed5ba2 Compare December 20, 2018 11:50
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-1367910 branch 6 times, most recently from 088bcb6 to a384ace Compare January 9, 2019 18:50
The behaviour of the request/release operations of web locks are
modified to be O(1) instead of their currently O(n) worst case runtime.
Additionally the query-order wpt is modified to reflect the new spec
requirement that the state returned by navigator.locks.query need only
respect ordering for requested locks per resource.

Bug: 913014
Change-Id: I819f8c27c995cb698a7c8b2c75ee80d32c744f07
Spec: https://wicg.github.io/web-locks/#algorithms
Reviewed-on: https://chromium-review.googlesource.com/c/1367910
Commit-Queue: Andreas Butler <[email protected]>
Reviewed-by: Victor Costan <[email protected]>
Reviewed-by: Joshua Bell <[email protected]>
Reviewed-by: Daniel Murphy <[email protected]>
Cr-Commit-Position: refs/heads/master@{#621833}
@lukebjerring
Copy link
Contributor

Blocked on chrome instability:

Test Subtest Results Messages
/web-locks/signal.tentative.https.any.serviceworker.html An aborted request results in AbortError FAIL: 9/10, PASS: 1/10 assert_equals: expected 1 but got 3;assert_equals: expected 1 but got 2
/web-locks/signal.tentative.https.any.serviceworker.html Abort after a timeout FAIL: 9/10, PASS: 1/10 assert_equals: expected 1 but got 3;assert_equals: expected 1 but got 2

Both of these results were already flaky:
https://staging.wpt.fyi/results/web-locks/signal.tentative.https.any.sharedworker.html?label=master&label=experimental&max-count=10&product=chrome&q=%28chrome%3Apass%7Cchrome%3Aok%29+%28chrome%3Atimeout%7Cchrome%3Aerror%7Cchrome%3Afail%29

@foolip
Copy link
Member

foolip commented Jan 23, 2019

I've investigated and filed #15015, pretty hopeful it can be fixed. The test in question wasn't changed by this PR, only affected by the change to helpers.js, so I'll go ahead and admin merge this PR.

@foolip foolip merged commit 27085bc into master Jan 23, 2019
@foolip foolip deleted the chromium-export-cl-1367910 branch January 23, 2019 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants