Using dimension_fields in models.ImageField cause performance penalties

[stan]

The main purpose of the height_field and width_field attributes is to
give a performance boost in some situations,
these fields acting as a cache on the dimensions of the image.

I recently have switched-on those attributes for a newspaper
publishing project where we manipulate some huge .tiff pictures
because there were big overloads and very slow responses for some
simple GET views displaying the image dimensions due to a sytematic
recalculation via the PIL.
Turning on width|heigh_field has dramaticly improved the performance
for these views. But as a side-effect, the POST context have taken the
absolute opposite direction in terms of responsiveness.

After digging a bit with pdb I found out that the
ImageFileDescriptor.__set__(self, instance, value) call in the form
(formset in my case) validation process was the reason.
When the dimension_fields are presents, they are filled-in with the
following call line 314 in db/models/fields/files.py :

self.field.update_dimension_fields(instance,
force=True)

where nothing happens when the fields are not presents (because we
don't need the dimensions of forms containing images that have not
changed).

Here a simple use-case to illustrate :

models.py
--------------

class Person(models.Model):
name = models.CharField(max_length=100, blank=True, null=True)


class ImagePerson(models.Model):
person = models.ForeignKey('Person')
image = models.ImageField('image', height_field='image_height',
width_field='image_width')
image_height = models.IntegerField(blank=True, null=True)
image_width = models.IntegerField(blank=True, null=True)


views/forms
----------------

class PersonForm(forms.ModelForm):
class Meta:
model = Person

class ImagePersonForm(forms.ModelForm):
class Meta:
model = ImagePerson
exclude = ('image_height', 'image_width',)

ImagePersonFormSet = inlineformset_factory(Person, ImagePerson,
form=ImagePersonForm)


# The view is a extremly simplified show/edit with an inline_form.

def detail_person(request, person_id):
person = Person.objects.get(pk=person_id)

person_form = PersonForm(request.POST or None, instance=person)
image_person_formset = ImagePersonFormSet(request.POST or None,
request.FILES or None,\ instance=person)

if request.method == 'POST':
pdb.set_trace()
if image_person_formset.is_valid():
image_person_formset.save()

return render_to_response('my_app/detail_person.html', {
'person_form': person_form,
'image_person_formset': image_person_formset,
}, context_instance=RequestContext(request))


use case
-------------

The simple fact of posting an unchanged form of a person cause the
opening with the PIL of *all* the unmodified images to fill-in the
dimension_fields of the virtual models in the form validation process.



This looks like an unecessary overhead and maybe something more lazy
could be more appropriate. I am sorry for not coming with a patch - I
do not get the whole picture of the core - but an expert point of
views is welcome here :-)

[Russell Keith-Magee]

On Mon, Apr 11, 2011 at 8:16 PM, stan <stanisla...@gmail.com> wrote:
> The main purpose of the height_field and width_field attributes is to
> give a performance boost in some situations,
> these fields acting as a cache on the dimensions of the image.

...

> The simple fact of posting an unchanged form of a person cause the
> opening with the PIL of *all* the unmodified images to fill-in the
> dimension_fields of the virtual models in the form validation process.
>
> This looks like an unecessary overhead and maybe something more lazy
> could be more appropriate. I am sorry for not coming with a patch -  I
> do not get the whole picture of the core - but an expert point of
> views is welcome here :-)

It sounds to me like you could be right; this strikes me as an
oversight, rather than anything deliberate. It's worth opening a
ticket so that this isn't forgotten.

Although you're seeing the problem with FormSets, I suspect you'll
find that it exists with normal forms, too -- it just isn't as
pronounced because a formset multiplies the scope of the problem.

Looking at the code in question, the reference to #11084 is a good
starting point for further analysis. It describes the exact problem
that you're talking about under a slightly different context. I
suspect you'll find that the fix to your problem will be in a similar
mould -- i.e., find a set of conditions that accurately identifies
when a form is being instantiated with existing file data, and ensure
that the dimension update doesn't happen under those conditions.

Yours,
Russ Magee %-)

[stan]

Thank Russ,

I have opened a ticket : http://code.djangoproject.com/ticket/15817

Unfortunately, I can't find a easy way to solve the problem because
the solution is somewhere in the ImageFileDesciptor.__set__() method
which is a dangerous area !

Cordialement,

Stanislas.

On Apr 12, 2:03 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote: