From 26fc561838f3ebd31d02e9f1438ff606acb9e6ed Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Wed, 11 Apr 2018 14:14:52 -0400 Subject: [PATCH 1/2] Allow RequestScheduler to throttle white-listed servers differently. Since HTTP/2 does not have the same performance characteristics/browser limit that HTTP/1.1 does, throttling to 6 requests per server does not make sense. However, disabling throttling completely is also a bad idea because we may end up making too many requests and throwing some away (for example camera flights loading terrain that is no longer in view). As a compromise, I arrived at 18 being a good request limit. I chose this number because many HTTP/1.1 data providers in Cesium use a/b/c subdomains to work around browser limits, so 3 * 6 = 18. By default, I've added the Cesium ion servers to this list, but we can and should add other known/popular HTTP/2 servers used by Cesium in the future. We'll eventually make this public whenever the rest of the RequestScheduler goes public. --- Source/Core/RequestScheduler.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Source/Core/RequestScheduler.js b/Source/Core/RequestScheduler.js index 10ccb51b9a0..6fd455b5800 100644 --- a/Source/Core/RequestScheduler.js +++ b/Source/Core/RequestScheduler.js @@ -2,6 +2,7 @@ define([ '../ThirdParty/Uri', '../ThirdParty/when', './Check', + './defaultValue', './defined', './defineProperties', './Event', @@ -13,6 +14,7 @@ define([ Uri, when, Check, + defaultValue, defined, defineProperties, Event, @@ -67,12 +69,21 @@ define([ RequestScheduler.maximumRequests = 50; /** - * The maximum number of simultaneous active requests per server. Un-throttled requests do not observe this limit. + * The maximum number of simultaneous active requests per server. Un-throttled requests or servers specifically + * listed in requestsByServer do not observe this limit. * @type {Number} * @default 6 */ RequestScheduler.maximumRequestsPerServer = 6; + /** + * A per serverKey list of overrides to use for throttling instead of maximumRequestsPerServer + */ + RequestScheduler.requestsByServer = { + 'api.cesium.com:443': 18, + 'assets.cesium.com:443': 18 + }; + /** * Specifies if the request scheduler should throttle incoming requests, or let the browser queue requests under its control. * @type {Boolean} @@ -146,7 +157,8 @@ define([ } function serverHasOpenSlots(serverKey) { - return numberOfActiveRequestsByServer[serverKey] < RequestScheduler.maximumRequestsPerServer; + var maxRequests = defaultValue(RequestScheduler.requestsByServer[serverKey], RequestScheduler.maximumRequestsPerServer); + return numberOfActiveRequestsByServer[serverKey] < maxRequests; } function issueRequest(request) { From ac36dd45de8346a5681ddfb5d8c86df1549cb136 Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Mon, 23 Apr 2018 15:38:43 -0400 Subject: [PATCH 2/2] Add test for request throttling override Also replace all usage of `foo.com` with `test.invalid`, since we should always be using `.invalid` as the tld for anything in tests. --- Specs/Core/RequestSchedulerSpec.js | 75 +++++++++++++++++++++--------- 1 file changed, 52 insertions(+), 23 deletions(-) diff --git a/Specs/Core/RequestSchedulerSpec.js b/Specs/Core/RequestSchedulerSpec.js index fbdf872694c..55f49fc1492 100644 --- a/Specs/Core/RequestSchedulerSpec.js +++ b/Specs/Core/RequestSchedulerSpec.js @@ -13,21 +13,25 @@ defineSuite([ var originalMaximumRequests; var originalMaximumRequestsPerServer; var originalPriorityHeapLength; + var originalRequestsByServer; beforeAll(function() { originalMaximumRequests = RequestScheduler.maximumRequests; originalMaximumRequestsPerServer = RequestScheduler.maximumRequestsPerServer; originalPriorityHeapLength = RequestScheduler.priorityHeapLength; + originalRequestsByServer = RequestScheduler.requestsByServer; }); beforeEach(function() { RequestScheduler.clearForSpecs(); + RequestScheduler.requestsByServer = {}; }); afterEach(function() { RequestScheduler.maximumRequests = originalMaximumRequests; RequestScheduler.maximumRequestsPerServer = originalMaximumRequestsPerServer; RequestScheduler.priorityHeapLength = originalPriorityHeapLength; + RequestScheduler.requestsByServer = originalRequestsByServer; }); it('request throws when request is undefined', function() { @@ -61,13 +65,13 @@ defineSuite([ }); it('getServer with https', function() { - var server = RequestScheduler.getServerKey('https://foo.com/1'); - expect(server).toEqual('foo.com:443'); + var server = RequestScheduler.getServerKey('https://test.invalid/1'); + expect(server).toEqual('test.invalid:443'); }); it('getServer with http', function() { - var server = RequestScheduler.getServerKey('http://foo.com/1'); - expect(server).toEqual('foo.com:80'); + var server = RequestScheduler.getServerKey('http://test.invalid/1'); + expect(server).toEqual('test.invalid:80'); }); it('honors maximumRequests', function() { @@ -84,7 +88,7 @@ defineSuite([ function createRequest() { return new Request({ - url : 'http://foo.com/1', + url : 'http://test.invalid/1', requestFunction : requestFunction, throttle : true }); @@ -147,7 +151,7 @@ defineSuite([ return deferred.promise; } - var url = 'http://foo.com/1'; + var url = 'http://test.invalid/1'; var server = RequestScheduler.getServerKey(url); function createRequest() { @@ -216,7 +220,7 @@ defineSuite([ function createRequest(priority) { var request = new Request({ - url : 'http://foo.com/1', + url : 'http://test.invalid/1', requestFunction : requestFunction, throttle : true, priority : priority @@ -302,7 +306,7 @@ defineSuite([ }); it('request goes through immediately when throttle is false', function() { - var url = 'https://foo.com/1'; + var url = 'https://test.invalid/1'; testImmediateRequest(url, false); }); @@ -317,7 +321,7 @@ defineSuite([ var request = new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); expect(request.state).toBe(RequestState.UNISSUED); @@ -342,7 +346,7 @@ defineSuite([ var request = new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); @@ -373,7 +377,7 @@ defineSuite([ var request = new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction, cancelFunction : cancelFunction }); @@ -409,7 +413,7 @@ defineSuite([ } var request = new Request({ - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); @@ -442,7 +446,7 @@ defineSuite([ function createRequest(priority) { return new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : getRequestFunction(priority), priority : priority }); @@ -477,7 +481,7 @@ defineSuite([ function createRequest(priority) { return new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction, priorityFunction : getPriorityFunction(priority) }); @@ -529,7 +533,7 @@ defineSuite([ function createRequest(priority) { return new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction, priority : priority }); @@ -566,7 +570,7 @@ defineSuite([ function createRequest(throttle) { return new Request({ - url : 'http://foo.com/1', + url : 'http://test.invalid/1', requestFunction : requestFunction, throttle : throttle }); @@ -604,7 +608,7 @@ defineSuite([ function createRequest(throttleByServer) { return new Request({ - url : 'http://foo.com/1', + url : 'http://test.invalid/1', requestFunction : requestFunction, throttleByServer : throttleByServer }); @@ -637,7 +641,7 @@ defineSuite([ RequestScheduler.throttleRequests = true; var request = new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); var promise = RequestScheduler.request(request); @@ -646,7 +650,7 @@ defineSuite([ RequestScheduler.throttleRequests = false; request = new Request({ throttle : true, - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); promise = RequestScheduler.request(request); @@ -669,7 +673,7 @@ defineSuite([ function createRequest() { return new Request({ - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); } @@ -708,7 +712,7 @@ defineSuite([ } var request = new Request({ - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); @@ -806,7 +810,7 @@ defineSuite([ } var request = new Request({ - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestFunction }); @@ -839,7 +843,7 @@ defineSuite([ } var requestToCancel = new Request({ - url : 'https://foo.com/1', + url : 'https://test.invalid/1', requestFunction : requestCancelFunction }); @@ -854,4 +858,29 @@ defineSuite([ cancelDeferred.resolve(); removeListenerCallback(); }); + + it('RequestScheduler.requestsByServer allows for custom maximum requests', function() { + var promise; + + RequestScheduler.requestsByServer['test.invalid:80'] = 23; + + for (var i = 0; i < 23; i++) { + promise = RequestScheduler.request(new Request({ + url: 'http://test.invalid/1', + throttle: true, + throttleByServer: true, + requestFunction: function() { return when.defer(); } + })); + RequestScheduler.update(); + expect(promise).toBeDefined(); + } + + promise = RequestScheduler.request(new Request({ + url: 'http://test.invalid/1', + throttle: true, + throttleByServer: true, + requestFunction: function() { return when.defer(); } + })); + expect(promise).toBeUndefined(); + }); });