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

Minor JavaScript Memory Leak: piwikApiService allRequests array #12105

Closed
jvilk opened this issue Sep 22, 2017 · 2 comments
Closed

Minor JavaScript Memory Leak: piwikApiService allRequests array #12105

jvilk opened this issue Sep 22, 2017 · 2 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Performance For when we could improve the performance / speed of Matomo. duplicate For issues that already existed in our issue tracker and were reported previously.

Comments

@jvilk
Copy link

jvilk commented Sep 22, 2017

Problem

The piwikApiService class contains an array called allRequests that it appends to whenever the frontend makes an API request. However, it only clears this array if abortAll is called. Over a session with the Piwik dashboard, the array continues to grow as the frontend makes API requests.

Solution

allRequests should only contain active requests that have not yet completed, as it is only used by abortAll.

A simple solution is to remove requests when they complete. To do this, change line 170 to the following:

allRequests.push(request);
promise.finally(function() {
  var index = allRequests.indexOf(request);
  if (index !== -1) {
    allRequests.splice(index, 1);
  }
});
@tsteur
Copy link
Member

tsteur commented Sep 23, 2017

Great suggestion. Do you mind issuing a pull request for this?

@tsteur tsteur added Bug For errors / faults / flaws / inconsistencies etc. c: Performance For when we could improve the performance / speed of Matomo. labels Sep 23, 2017
jvilk pushed a commit to jvilk/piwik that referenced this issue Nov 2, 2017
jvilk pushed a commit to jvilk/piwik that referenced this issue Nov 8, 2017
@jvilk
Copy link
Author

jvilk commented Nov 9, 2017

PR issued and awaiting review!

tsteur pushed a commit that referenced this issue Dec 14, 2017
@mattab mattab added the duplicate For issues that already existed in our issue tracker and were reported previously. label Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Performance For when we could improve the performance / speed of Matomo. duplicate For issues that already existed in our issue tracker and were reported previously.
Projects
None yet
Development

No branches or pull requests

3 participants