Max origin concurrent auctions #2743
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jaiminpanchal27
approved these changes
Jun 20, 2018
mkendall07
reviewed
Jun 20, 2018
| @@ -74,6 +74,11 @@ events.on(CONSTANTS.EVENTS.BID_ADJUSTMENT, function (bid) { | |||
| adjustBids(bid); | |||
| }); | |||
|
|
|||
| const MAX_REQUESTS_PER_ORIGIN = 4; | |||
There was a problem hiding this comment.
what's the magic behind 4? I thought modern browser was 6?
There was a problem hiding this comment.
On my specific test page there wasn't much difference with fast internet between 4 and 6 but there was a slight improvement on slow pages with 4 vs 6. I'm not sure why, but I decided it would be safer to go with that value; if other's test prove differently then we could change it down the line. It's also one of the reasons I made it configurable.
mkendall07
approved these changes
Jun 20, 2018
| sourceInfo[source].SRA = false; | ||
| } | ||
| }, | ||
| done(origin) { |
There was a problem hiding this comment.
Should we rename this done to avoid confusion between auction done and this done ?
There was a problem hiding this comment.
we could, although I'm not sure it's a huge deal. I intentionally kept it as short as possible since it's an object property, so it won't be minified.
dluxemburg
pushed a commit
to Genius/Prebid.js
that referenced
this pull request
Jul 17, 2018
* initial attempt at limiting concurrenet auctions by origin * fix queueing of auctions for max origin * don't decrement on timeout as it is already called by onreadystatechange * move auction timer so it doesn't start until queued auction starts * set default max concurrent origin requests to 4 and make configurable * fix tests to not queue for auction.callBids * change MAX_REQUEST_PER_ORIGIN to local var
florevallatmrf
pushed a commit
to Marfeel/Prebid.js
that referenced
this pull request
Sep 6, 2018
* initial attempt at limiting concurrenet auctions by origin * fix queueing of auctions for max origin * don't decrement on timeout as it is already called by onreadystatechange * move auction timer so it doesn't start until queued auction starts * set default max concurrent origin requests to 4 and make configurable * fix tests to not queue for auction.callBids * change MAX_REQUEST_PER_ORIGIN to local var
StefanWallin
pushed a commit
to mittmedia/Prebid.js
that referenced
this pull request
Sep 28, 2018
* initial attempt at limiting concurrenet auctions by origin * fix queueing of auctions for max origin * don't decrement on timeout as it is already called by onreadystatechange * move auction timer so it doesn't start until queued auction starts * set default max concurrent origin requests to 4 and make configurable * fix tests to not queue for auction.callBids * change MAX_REQUEST_PER_ORIGIN to local var
ghost
pushed a commit
to devunrulymedia/Prebid.js
that referenced
this pull request
Jan 30, 2019
* initial attempt at limiting concurrenet auctions by origin * fix queueing of auctions for max origin * don't decrement on timeout as it is already called by onreadystatechange * move auction timer so it doesn't start until queued auction starts * set default max concurrent origin requests to 4 and make configurable * fix tests to not queue for auction.callBids * change MAX_REQUEST_PER_ORIGIN to local var
AlessandroDG
pushed a commit
to simplaex/Prebid.js
that referenced
this pull request
Mar 26, 2019
* initial attempt at limiting concurrenet auctions by origin * fix queueing of auctions for max origin * don't decrement on timeout as it is already called by onreadystatechange * move auction timer so it doesn't start until queued auction starts * set default max concurrent origin requests to 4 and make configurable * fix tests to not queue for auction.callBids * change MAX_REQUEST_PER_ORIGIN to local var
AlessandroDG
pushed a commit
to simplaex/Prebid.js
that referenced
this pull request
Mar 26, 2019
* initial attempt at limiting concurrenet auctions by origin * fix queueing of auctions for max origin * don't decrement on timeout as it is already called by onreadystatechange * move auction timer so it doesn't start until queued auction starts * set default max concurrent origin requests to 4 and make configurable * fix tests to not queue for auction.callBids * change MAX_REQUEST_PER_ORIGIN to local var
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
Description of change
In testing I was seeing poorer performance with 1-x (compared to 0.34) when concurrent auctions (simultaneous
pbjs.requestBids()) were running on slow connection. 1-x is concurrent by default, and 0.34 is never concurrent.A lot of concurrency issues seemed to be related to max requests per origin limits in browsers. This pull-request adds a queuing feature for auctions that will queue auctions that would put the client over the limit of requests per origin (with that limit defaulting to
4and being configurable withconfig.setConfig({maxRequestsPerOrigin: 6})). Setting themaxRequestsPerOriginto1will effectively emulate the behavior in 0.34 by having all concurrent auctions be queued until the previous auction has finished.Comparison of results before and after this pull-request change on 1-x for a fast connection. (stacked requests from first example go off bottom of screen whereas second example shows all requests)
Speeds are very similar between each other and both results are very fast compared to 0.34 (1-x total time for requests in both examples above about ~1s, but ~3.5 seconds for 0.34 below due to no concurrency)
Comparison of results on slow connection in 1-x before and after max concurrency per origin implemented.
Total time of all auctions on left is 3.5s, but without max concurrency per origin timed-out bids cause further auctions to miss their boat. Much improved on the right with limited requests per origin, total time of 7s for all auctions (as some are queued) but get back 3 times as many bids.
Other information
Related to #2648.
Also cleaned up the ajax module to remove
XDomainRequestsupport asIE9is no longer supported by Prebid.js.