Accounts seems to be hit a lot! #2892
Comments
|
One potential solution would be to debounce the $http request by something like 100ms. We could use the angular-debounce library to do that for us. Note that, if we did such a thing, it might affect how our unit tests go. We would need to make sure the debounce library was properly flushing during unit tests to have the same behavior. |
|
I'll tackle this one, since I am on a performance kick. ⚽ |
|
@jniles you just beat me to it! I haven't started yet though so all yours. At one point there was an effort to use client side 'Stores' that could be shared across components and only make a request if they didn't yet have data available, I wonder if something like that would be useful. The main consideration with that would be when is it appropriate to invalidate data and check again. This looks to me like component initialisation requests that make fresh requests on initialisation and aren't aware of any other component. |
|
Yeah, I'm almost 100% sure that is the issue as well. Check this line out - it creates 300 bhAccountSelect components. I'll have a PR in under an hour that you can look at to see how it does. |
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
|
@jniles I've just had a follow up thought on this. In cases like you have demonstrated/ linked here the reason so many requests are made is because the account component is responsible for fetching its own accounts and we are not always careful about where we place it. What if we gave the component a On large complex pages that know there will be hundreds of component instances the programmer could simply prefer <bh-account-select accounts="MyCtrl.accounts"></bh-account-select>Over: <bh-account-select fetch></bh-account-select>Potentially the ideal end solution involves client side caching and TTL etc. however this might be a good middle ground, it leaves the controller developer more responsible but would limit one module to one request. |
|
@sfount good suggestion! The simplicity of a fetch tag is probably better than the complexity of what I was working with. The downside is that we then have to identify all the areas where accounts is hit a lot and make sure we manually manage the accounts I'll take a look at incorporating these ideas and see how they work. On the short term at least, this will relieve some pressure on the heavily used pages. |
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
This commit implements the HttpCacheService, a new wrapper for any $http get request that will cache the result for a specified number of seconds to ensure that large payloads are not called too frequently and overload the server. A prototype is implement on the Accounts Service. Closes Third-Culture-Software#2892.
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
This commit implements the HttpCacheService, a new wrapper for any $http get request that will cache the result for a specified number of seconds to ensure that large payloads are not called too frequently and overload the server. A prototype is implement on the Accounts Service. Closes Third-Culture-Software#2892.
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
This commit implements the HttpCacheService, a new wrapper for any $http get request that will cache the result for a specified number of seconds to ensure that large payloads are not called too frequently and overload the server. A prototype is implement on the Accounts Service. Closes Third-Culture-Software#2892.
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
This commit implements the HttpCacheService, a new wrapper for any $http get request that will cache the result for a specified number of seconds to ensure that large payloads are not called too frequently and overload the server. A prototype is implement on the Accounts Service. Closes Third-Culture-Software#2892.
This commit adds a debounce() call to the read request. Since it uses promises and Angular's $timeout(), it will require flushing the $timeouts in tests. Closes Third-Culture-Software#2892.
This commit implements the HttpCacheService, a new wrapper for any $http get request that will cache the result for a specified number of seconds to ensure that large payloads are not called too frequently and overload the server. A prototype is implement on the Accounts Service. Closes Third-Culture-Software#2892.
3111: Perf: implement HttpCache service to cache Accounts.read() call r=jniles a=jniles
**Original Text**
~~This PR introduces a new factory function for debouncing function calls. The suggested ng-debounce library did not use promises and was cumbersome to use, thus I have implemented my own based on a SO issue. The `Accounts.read()` method has been debounced by 2000 milliseconds to allow for things like `bhAccountSelects` to benefit from the same HTTP request instead of making one each.~~
**Updated Information**
This Pull request implements a new service called `HttpCache` that should transparently wrap any HTTP requests and ensure that the same function call will return the same result. This is useful for smaller, more frequently called routes.
_Basic Usage:_
```js
// a GET HTTP request to some url
function read(id, options) {
return $http.get(/*...*/);
}
// to proxy, simply wrap the function in HttpCache
const proxy = HttpCache((id, options) => read(id, options));
// You can now call proxy as
proxy(1, { limit : 10 })
.then(() => /*... */);
```
For uncached requests, the behavior is the same a raw HTTP request. For those that have been called multiple times, the web page no longer needs to send an extra HTTP request.
If you want to skip the cache (and bust it), simply pass `true` as the third parameter.
Closes #2892.
Co-authored-by: Jonathan Niles <[email protected]>
Co-authored-by: Jonathan Niles <[email protected]>
It looks like our HTTP requests seems to be hitting the
/accounts/route a lot! We should see if there is any way to reduce these requests - from my naive guess, the following screenshot was taken from the stock module.The text was updated successfully, but these errors were encountered: