Message ID | 1468403849-7325-1-git-send-email-stephen.finucane@intel.com |
---|---|
State | Accepted |
Headers | show |
On 07/13/2016 04:57 AM, Stephen Finucane wrote: > The way that reverse indexing of patches was implemented is broken. > At present, it will retrieve all patches in memory and return the > length from that data, then the slicing operation will then happen > without querying the DB and slice the results cached from the len() > evaluation. This is memory intensive, particularly for larger > instances. > > Take advantage of Django's lazy loading to avoid this. > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > Suggested-by: Damien Lespiau <damien.lespiau@intel.com> > --- Reviewed-by: Andy Doan <andy.doan@linaro.org> > patchwork/views/xmlrpc.py | 14 ++++++++------ > 1 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py > index 638b3b1..8f759aa 100644 > --- a/patchwork/views/xmlrpc.py > +++ b/patchwork/views/xmlrpc.py > @@ -406,8 +406,8 @@ def project_list(search_str=None, max_count=0): > if max_count > 0: > return list(map(project_to_dict, projects[:max_count])) > elif max_count < 0: > - return list(map(project_to_dict, > - projects[len(projects) + max_count:])) > + query = projects.reverse()[:-max_count] > + return list(map(project_to_dict, reversed(query))) > else: > return list(map(project_to_dict, projects)) > except Project.DoesNotExist: > @@ -461,7 +461,8 @@ def person_list(search_str=None, max_count=0): > if max_count > 0: > return list(map(person_to_dict, people[:max_count])) > elif max_count < 0: > - return list(map(person_to_dict, people[len(people) + max_count:])) > + query = people.reverse()[:-max_count] > + return list(map(person_to_dict, reversed(query))) > else: > return list(map(person_to_dict, people)) > except Person.DoesNotExist: > @@ -606,8 +607,8 @@ def patch_list(filt=None): > if max_count > 0: > return list(map(patch_to_dict, patches[:max_count])) > elif max_count < 0: > - return list(map(patch_to_dict, > - patches[len(patches) + max_count:])) > + query = patches.reverse()[:-max_count] > + return list(map(patch_to_dict, reversed(query))) > else: > return list(map(patch_to_dict, patches)) > except Patch.DoesNotExist: > @@ -802,7 +803,8 @@ def state_list(search_str=None, max_count=0): > if max_count > 0: > return list(map(state_to_dict, states[:max_count])) > elif max_count < 0: > - return list(map(state_to_dict, states[len(states) + max_count:])) > + query = states.reverse()[:-max_count] > + return list(map(state_to_dict, reversed(query))) > else: > return list(map(state_to_dict, states)) > except State.DoesNotExist: >
On 15 Jul 11:31, Andy Doan wrote: > On 07/13/2016 04:57 AM, Stephen Finucane wrote: > > The way that reverse indexing of patches was implemented is broken. > > At present, it will retrieve all patches in memory and return the > > length from that data, then the slicing operation will then happen > > without querying the DB and slice the results cached from the len() > > evaluation. This is memory intensive, particularly for larger > > instances. > > > > Take advantage of Django's lazy loading to avoid this. > > > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > > Suggested-by: Damien Lespiau <damien.lespiau@intel.com> > > --- > Reviewed-by: Andy Doan <andy.doan@linaro.org> Merged.
diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py index 638b3b1..8f759aa 100644 --- a/patchwork/views/xmlrpc.py +++ b/patchwork/views/xmlrpc.py @@ -406,8 +406,8 @@ def project_list(search_str=None, max_count=0): if max_count > 0: return list(map(project_to_dict, projects[:max_count])) elif max_count < 0: - return list(map(project_to_dict, - projects[len(projects) + max_count:])) + query = projects.reverse()[:-max_count] + return list(map(project_to_dict, reversed(query))) else: return list(map(project_to_dict, projects)) except Project.DoesNotExist: @@ -461,7 +461,8 @@ def person_list(search_str=None, max_count=0): if max_count > 0: return list(map(person_to_dict, people[:max_count])) elif max_count < 0: - return list(map(person_to_dict, people[len(people) + max_count:])) + query = people.reverse()[:-max_count] + return list(map(person_to_dict, reversed(query))) else: return list(map(person_to_dict, people)) except Person.DoesNotExist: @@ -606,8 +607,8 @@ def patch_list(filt=None): if max_count > 0: return list(map(patch_to_dict, patches[:max_count])) elif max_count < 0: - return list(map(patch_to_dict, - patches[len(patches) + max_count:])) + query = patches.reverse()[:-max_count] + return list(map(patch_to_dict, reversed(query))) else: return list(map(patch_to_dict, patches)) except Patch.DoesNotExist: @@ -802,7 +803,8 @@ def state_list(search_str=None, max_count=0): if max_count > 0: return list(map(state_to_dict, states[:max_count])) elif max_count < 0: - return list(map(state_to_dict, states[len(states) + max_count:])) + query = states.reverse()[:-max_count] + return list(map(state_to_dict, reversed(query))) else: return list(map(state_to_dict, states)) except State.DoesNotExist:
The way that reverse indexing of patches was implemented is broken. At present, it will retrieve all patches in memory and return the length from that data, then the slicing operation will then happen without querying the DB and slice the results cached from the len() evaluation. This is memory intensive, particularly for larger instances. Take advantage of Django's lazy loading to avoid this. Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> Suggested-by: Damien Lespiau <damien.lespiau@intel.com> --- patchwork/views/xmlrpc.py | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)