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

Spec: group-by-origin execution mode. #905

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

qingxinwu
Copy link
Collaborator

@qingxinwu qingxinwu commented Nov 14, 2023

Spec IG's group-by-origin execution mode, which attempt to re-use the execution environment for interest groups with the same script that were joined on the same top-level origin, which saves a lot of these initialization costs.


💥 Error: 400 Bad Request 💥

PR Preview failed to build. (Last tried on Mar 5, 2024, 4:47 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 [Related URL]([object Object])

Error running preprocessor, returned code: 2.
FATAL ERROR: '&eq; isn't a valid character reference.
WARNING: The var 'signalsUrl' (in algorithm 'build bid generators map') is only used once.
If this is not a typo, please add an ignore='' attribute to the <var>.
WARNING: The var 'slotSizeQueryParam' (in algorithm 'build bid generators map') is only used once.
If this is not a typo, please add an ignore='' attribute to the <var>.
WARNING: The var 'joiningOrigin' (in algorithm 'build bid generators map') is only used once.
If this is not a typo, please add an ignore='' attribute to the <var>.
WARNING: The var 'key' (in algorithm 'build bid generators map') is only used once.
If this is not a typo, please add an ignore='' attribute to the <var>.
WARNING: The var 'sortedAndGroupedIgs' (in algorithm 'build bid generators map') is only used once.
If this is not a typo, please add an ignore='' attribute to the <var>.
Fetching issue   1/1: WICG/turtledove/522
LINK ERROR: Multiple possible 'TypeError' idl refs.
Arbitrarily chose https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-native-error-types-used-in-this-standard-typeerror
To auto-select one of the following refs, insert one of these lines into a <pre class=link-defaults> block:
spec:ecmascript; type:exception; text:TypeError
spec:webidl; type:exception; text:TypeError
{{TypeError}}
LINK ERROR: Multiple possible 'code unit' dfn refs.
Arbitrarily chose https://infra.spec.whatwg.org/#code-unit
To auto-select one of the following refs, insert one of these lines into a <pre class=link-defaults> block:
spec:infra; type:dfn; text:code unit
spec:i18n-glossary; type:dfn; text:code unit
[=code unit=]
Unexported dfn that's not referenced locally - did you mean to export it?
<dfn bs-line-number="5041" data-dfn-type="dfn" id="per-buyer-bid-generator" data-lt="per buyer bid generator" data-noexport="by-default" class="dfn-paneled">per buyer bid generator</dfn>
Unexported dfn that's not referenced locally - did you mean to export it?
<dfn bs-line-number="5050" data-dfn-type="dfn" data-dfn-for="script runner key" id="script-runner-key-script-url" data-lt="script url" data-noexport="by-default" class="dfn-paneled">script url</dfn>
Unexported dfn that's not referenced locally - did you mean to export it?
<dfn bs-line-number="5052" data-dfn-type="dfn" data-dfn-for="script runner key" id="script-runner-key-signals-url" data-lt="signals url" data-noexport="by-default" class="dfn-paneled">signals url</dfn>
Unexported dfn that's not referenced locally - did you mean to export it?
<dfn bs-line-number="5054" data-dfn-type="dfn" data-dfn-for="script runner key" id="script-runner-key-experiment-group-id" data-lt="experiment group id" data-noexport="by-default" class="dfn-paneled">experiment group id</dfn>
Unexported dfn that's not referenced locally - did you mean to export it?
<dfn bs-line-number="5056" data-dfn-type="dfn" data-dfn-for="script runner key" id="script-runner-key-trusted-bidding-signals-slot-size-param" data-lt="trusted bidding signals slot size param" data-noexport="by-default" class="dfn-paneled">trusted bidding signals slot size param</dfn>
 ✘  Did not generate, due to errors exceeding the allowed error level.

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@qingxinwu qingxinwu added the spec Relates to the spec label Nov 14, 2023
@qingxinwu qingxinwu changed the title Spec group-by-origin execution mode. Spec: group-by-origin execution mode. Nov 14, 2023
@qingxinwu
Copy link
Collaborator Author

qingxinwu commented Nov 28, 2023

@morlovich mind reviewing this? Select "hide whitespace" in the files changed tab will make the PR easier to review.

@qingxinwu
Copy link
Collaborator Author

@morlovich friendly ping

@@ -1356,6 +1356,17 @@ To <dfn>parse an https origin</dfn> given a [=string=] |input|:
<div algorithm>

To <dfn>build bid generators map</dfn> given an [=auction config=] |auctionConfig|:

<div class="note"> This algorithm builds an [=ordered map=] with:
* key: An [=origin=], which is a buyer of [=auction config/interest group buyers=]
Copy link
Collaborator

Choose a reason for hiding this comment

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

May this be easier to read with a tuple key? Though I guess how we only care about joining origin some of the time may make it messy

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah really need to find a better way to do this. It's getting really complicated now with many layers, which is hard to understand and hard to maintain. It can go to a separate PR, but any suggestion will be appreciated.

spec.bs Outdated
1. [=map/For each=] |joiningOrigin| → |perBiddingUrlGenerator| of |perSignalsUrlGenerator|:
1. [=map/For each=] biddingUrl → |groups| of |perBiddingUrlGenerator|:
1. Let |realmForOriginGroupMode| be null.
1. [=list/For each=] |ig| of |groups|:
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are not that definitive in grouping, we do it opportunistically and order things based on priority... actually is there priority-ordering in the spec?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is it step 6 of https://wicg.github.io/turtledove/#apply-interest-groups-limits-to-prioritized-list? But seems we don't have the part of calculating priority from priority vector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec Relates to the spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants