Code

Opened 7 years ago

Closed 6 years ago

#6217 closed (fixed)

(u)gettext_lazy is a memory hog

Reported by: Jakub Wilk <ubanus@…> Owned by: mtredinnick
Component: Internationalization Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Objects created by a gettext_lazy/ugettext_lazy call occupies almost 12 KiB of memory.
This makes Django pretty unusable in an i18n environment with limited memory.

Here's a proof:

>>> import guppy
>>> heapy = guppy.hpy()
>>> heapy.heap()
Partition of a set of 68757 objects. Total size = 4651296 bytes.
  Index  Count   %     Size   % Cumulative  % Kind (class / dict of class)
      0  25728  37  1694752  36   1694752  36 str
      1  16723  24   615380  13   2310132  50 tuple
      2   7201  10   403256   9   2713388  58 function
      3    521   1   274376   6   2987764  64 dict (no owner)
      4    197   0   249512   5   3237276  70 dict of module
      5   3516   5   239088   5   3476364  75 types.CodeType
      6    474   1   206060   4   3682424  79 type
      7    474   1   189072   4   3871496  83 dict of type
      8   7623  11   182952   4   4054448  87 __builtin__.cell
      9    218   0   102608   2   4157056  89 dict of class
<269 more rows. Type e.g. '_.more' to view.>
>>> from django.utils.translation import ugettext_lazy
>>> tmp = [ugettext_lazy('foo%d' % i) for i in xrange(1000)]
>>> heapy.heap()
Partition of a set of 320759 objects. Total size = 16691776 bytes.
  Index  Count   %     Size   % Cumulative  % Kind (class / dict of class)
      0  68201  21  3819256  23   3819256  23 function
      1 129623  40  3110952  19   6930208  42 __builtin__.cell
      2  78723  25  2595380  16   9525588  57 tuple
      3   3521   1  2218760  13  11744348  70 dict (no owner)
      4   1054   0  1762288  11  13506636  81 dict of django.utils.functional.__proxy__
      5  26728   8  1726712  10  15233348  91 str
      6    197   0   249512   1  15482860  93 dict of module
      7   3516   1   239088   1  15721948  94 types.CodeType
      8    474   0   206060   1  15928008  95 type
      9    474   0   189072   1  16117080  97 dict of type
<269 more rows. Type e.g. '_.more' to view.>

Attachments (1)

django-lazy-piglet.diff (3.6 KB) - added by Jakub Wilk <ubanus@…> 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by Simon Greenhill <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Any hints on optimisation would be welcome.

comment:2 Changed 6 years ago by Jakub Wilk <ubanus@…>

The solution is to create method for every __proxy__ class, rather than for every __proxy__ instance.
I've also fixed some misleading comments in django.utils.functional.

Changed 6 years ago by Jakub Wilk <ubanus@…>

comment:3 Changed 6 years ago by Simon Greenhill <dev@…>

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

comment:4 follow-up: Changed 6 years ago by mtredinnick

  • Owner changed from nobody to mtredinnick
  • Triage Stage changed from Ready for checkin to Accepted

Not ready for checkin, since it's not Python 2.3 compatible. I also want to think about whether this is the best approach here, given a couple of other problems we've been seeing as this code gets used more and more.

comment:5 in reply to: ↑ 4 Changed 6 years ago by Piotr Lewandowski <django@…>

Replying to mtredinnick:

Not ready for checkin, since it's not Python 2.3 compatible.

Is this incompatibility related only to use of decorator syntax? If so, it could be easy fixed.

I also want to think about whether this is the best approach here, given a couple of other problems we've been seeing as this code gets used more and more.

What are those problems you've mentioned? I've been using it for a quite long time and it has been working flawlessly so far.

comment:6 Changed 6 years ago by mtredinnick

The Python 2.3 compatibility is obviously the decorators.

The main problem with the code is that it doesn't go far enough by taking into account the common usage patterns. Since we are almost always using this code with a single return type of "unicode" there's huge potential for reusing the class each time and caching it, rather than recreating it.

I've got a rewrite mostly done that uses a factory to create a class based on the return type (and returns the cached version if we've already done it) which seems a bit faster on startup.

comment:7 Changed 6 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [8120]) Fixed #6217 -- Reduced the memory used by each ugettext_lazy() call.
Thanks to Jakub Wilk for analysis and the initial patch.

comment:8 Changed 6 years ago by mtredinnick

(In [8126]) Back out [8120] for a bit. Refs #6217. It's having unintended side-effects (a.k.a breaks the tree)

comment:9 Changed 6 years ago by mtredinnick

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:10 Changed 6 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [8127]) Put back [8120] along with a small tweak. Fixed #6217.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.