newforms-admin raw_id_fields performance problem
Karen Tracey
conversion went fine, except for a performance problem with the new
raw_id_fields. As background, I have a model with two ForeignKey
fields (among other stuff). The number of choices for those
ForeignKey fields is high (over 100,000 for one of them). In the old
admin, I specified raw_id_admin=True on those fields in the model,
and all was well (not pretty, but usable). In the new admin, I
included these fields in raw_id_fields for the associated ModelAdmin
object. I know I specified it correctly because on the change form
for the model these two fields are displayed "raw-ly". However, the
change page for an instance of this model takes minutes to
display. Tracing through the code a little I can see that for each
of these fields a list of all possible values is getting generated
when the model's ChangeManipulator is created. That's exactly what I
was using raw_id_admin in the old version to prevent, because it
takes too long. The replacement raw_id_fields isn't avoiding that --
it's just suppressing the inclusion of a humongous drop-down list on
the change form.
Is there some way I've overlooked in the new admin to avoid the
overhead of generating the choices list for a field listed in
raw_id_fields? If not, is this a known problem that will be
addressed before newforms-admin gets merged back to trunk (I did
search in trac, but didn't find anything)? If not, should I open a ticket?
Thanks,
Karen
Malcolm Tredinnick
> I tried out the newforms-admin branch for my app today. The
> conversion went fine, except for a performance problem with the new
> raw_id_fields. As background, I have a model with two ForeignKey
> fields (among other stuff). The number of choices for those
> ForeignKey fields is high (over 100,000 for one of them). In the old
> admin, I specified raw_id_admin=True on those fields in the model,
> and all was well (not pretty, but usable). In the new admin, I
> included these fields in raw_id_fields for the associated ModelAdmin
> object. I know I specified it correctly because on the change form
> for the model these two fields are displayed "raw-ly". However, the
> change page for an instance of this model takes minutes to
> display. Tracing through the code a little I can see that for each
> of these fields a list of all possible values is getting generated
> when the model's ChangeManipulator is created.
This raises some alarm bells. Why on earth are we futzing around in
ChangeManipulators here? They are entirely oldforms-related and the
whole point is to remove any reliance on them.
This might well be a bug and in an area that just hasn't been addressed
yet, since I'd wager not a lot of regular testing is going on with
100,000 objects in those cases. Probably worth a ticket, Karen.
Regards,
Malcolm
Joseph Kocherhans
>
> This raises some alarm bells. Why on earth are we futzing around in
> ChangeManipulators here? They are entirely oldforms-related and the
> whole point is to remove any reliance on them.
>
> This might well be a bug and in an area that just hasn't been addressed
> yet, since I'd wager not a lot of regular testing is going on with
> 100,000 objects in those cases. Probably worth a ticket, Karen.
Yeah, there is some hideousness left that I forgot about. Specifically
here [1] and here [2]. There are actually quite a few places where
newforms-admin stuff calls out to old code that kinda works, but needs
to be rewritten. I have not done much scouring yet to clean all of
this up. It'll probably be November before I have a lot of time to
work on it again though :( Really, the render_change_form function
needs to be moved into the ModelAdmin class and the manipulator stuff
needs to be removed from it. Patches are welcome. (And no Malcolm,
that's not directed at you ;)
Joseph
Brian Rosner
>
> On 10/9/07, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
>>
>> This raises some alarm bells. Why on earth are we futzing around in
>> ChangeManipulators here? They are entirely oldforms-related and the
>> whole point is to remove any reliance on them.
>>
>> This might well be a bug and in an area that just hasn't been addressed
>> yet, since I'd wager not a lot of regular testing is going on with
>> 100,000 objects in those cases. Probably worth a ticket, Karen.
>
> Yeah, there is some hideousness left that I forgot about. Specifically
> here [1] and here [2]. There are actually quite a few places where
> newforms-admin stuff calls out to old code that kinda works, but needs
> to be rewritten. I have not done much scouring yet to clean all of
> this up. It'll probably be November before I have a lot of time to
> work on it again though :( Really, the render_change_form function
> needs to be moved into the ModelAdmin class and the manipulator stuff
> needs to be removed from it. Patches are welcome. (And no Malcolm,
> that's not directed at you ;)
Your wish is my command ;)
http://code.djangoproject.com/ticket/5720
>
> Joseph
>
> [1]
> http://code.djangoproject.com/browser/django/branches/newforms-admin/django/contrib/admin/options.py#L503
--
Brian Rosner
http://www.brosner.com/blog
Karen Tracey
>Your wish is my command ;)
>
>http://code.djangoproject.com/ticket/5720
Wow, that was fast. I can confirm this patch makes bringing up the
change form for my problematic model nice and zippy. Thanks!
I did run into another problem -- instances of these models are
commonly edited inline on the change page for a different
model. Bringing up a change or add page for that model had the same
performance issue until I realized that I needed to specify the
raw_id_fields in the "inline" admin class as well. Which makes
perfect sense and I see gives more flexibility than the old way, it
just wasn't immediately apparent to me.
Many thanks for the quick responses,
Karen
Joseph Kocherhans
>
> On 2007-10-09 14:39:57 -0600, "Joseph Kocherhans" <jkoch...@gmail.com> said:
>
> >
> > On 10/9/07, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
> >>
> >> This raises some alarm bells. Why on earth are we futzing around in
> >> ChangeManipulators here? They are entirely oldforms-related and the
> >> whole point is to remove any reliance on them.
> >>
> >> This might well be a bug and in an area that just hasn't been addressed
> >> yet, since I'd wager not a lot of regular testing is going on with
> >> 100,000 objects in those cases. Probably worth a ticket, Karen.
> >
> > Yeah, there is some hideousness left that I forgot about. Specifically
> > here [1] and here [2]. There are actually quite a few places where
> > newforms-admin stuff calls out to old code that kinda works, but needs
> > to be rewritten. I have not done much scouring yet to clean all of
> > this up. It'll probably be November before I have a lot of time to
> > work on it again though :( Really, the render_change_form function
> > needs to be moved into the ModelAdmin class and the manipulator stuff
> > needs to be removed from it. Patches are welcome. (And no Malcolm,
> > that's not directed at you ;)
>
> Your wish is my command ;)
>
> http://code.djangoproject.com/ticket/5720
Man, now I have to put my money where my mouth is. :)
Thank you much. Committed in [6470].
Just FYI I left the index view there. The patch had removed it. Yes,
it should die, but it's death belongs in a separate commit. I'm just
going to leave it there until I get around to ripping out a bunch of
other stuff that is probably no longer used.
Joseph