Hi all,
On Fri, Jun 24, 2011 at 12:38 PM, Gregor Müllegger <gre...@muellegger.de> wrote:
> Hi Jacob,
>
> 2011/6/23 Jacob Kaplan-Moss <ja...@jacobian.org>:
>> Hi Idan et al. --
>>
>> Thanks for putting this all together!
>>
>> In general, I like this a lot, and I'm always going to defer to the
>> eyes of someone like Idan who spends more time wrangling templates
>> than I do. So I like the general gist, and I most don't mind the {%
>> formconfig %} business.
>>
>> However, I do have a few concerns:
>>
>> 1. Performance: it looks, to me, like rending a basic form is going to
>> cause dozens of template includes and dozens of sub-renders (the form
>> loads a form template which loads row templates which load widget
>> templates). That's dozens of disk hits, and a lot of overhead for form
>> rendering. I worry about this overhead a lot. Django's performance has
>> slipped lately, and I'm really afraid this'll make things a lot worse.
>>
>> So I'm going to need to see some benchmarks -- particularly in how a
>> simple {% form myform %} compares to {{ form.as_* }}.
>>
>> The wrong performance benchmarks could result in a veto from me; this
>> is important.
>
> Unfortunatelly we don't have any implementation yet, except of what Bruno did
> with floppyforms, what we can benchmark. But I think we could minimize disk
> hits or template-loader work by caching the rendered templates ourself. For a
> form with many fields it's very likely that most of them use the same row
> level template, so we can reuse this a couple of times during the rendering. I
> will do my best to make things as fast as possible. Benchmark was not yet part
> of my GSoC timeline but I agree that this should have a high priority.
Someone did a couple of benchmarks to measure the performance impact
of my patch (#15667, the template-widgets implementation aka
django-floppyforms). There is some slowdown, the question is whether
it's acceptable or not, and where's the limit. If you have simple
forms, they'll render in 6 milliseconds instead of 3.5. However, with
fields with lots of choices, this gets much worse. Something that
takes 32 milliseconds with the current forms implementation can take
almost 200 milliseconds to render if it's done in the templates. The
biggest part of this time is spent actually rendering the templates,
not loading them. So template caching helps but only to a limited
extent.
Also see on the ticket page, there are a couple of pretty graphs that
show exactly where time is spent. For instance, 12% of the rendering
time is spent doing isinstance() calls, that's part of the template
rendering logic.
I did the same benchmarks with pypy, and the results are interesting.
Raw results are here, for some context see the files attached to the
ticket:
http://dpaste.com/hold/558425/
So, currently I see how it can be a problem to have this as the only
widget rendering system. I don't think it's worth maintaining both
implementation (the current one and the template one) with ways to
switch from one way to another, it's going to be too much of a burden
to maintain.
When the template language gets faster, maybe we can integrate my
patch. The question is, what slowdown is acceptable? Or are we
expecting a speedup? With jinja2, widget rendering is actually
*faster* using templates than using the current string-based
implementation but I'm not sure we can go this far with django
templates. So:
* We need faster templates for this to land in trunk
* We need pypy :)
* If you want template-base widgets *now*, use django-floppyforms.
* If you want to use the new forms / templates API as soon as it's
done… how do you do it? Is it going to be packaged as an app, as a
patched version of django?
That being said, I really like the design work that's been done and I
look forward to trying it.
-Bruno
> However based on the feedback I got on djangocon and on the mailinglist yet,
> the anticipation for this feature is really big. And we won't make code slower
> that already exists. You only get a bit slower form rendering if you are going
> to use the new mechanics, but you trade that for a much more flexible and
> faster template designing. And as Benoît described in the current thread: Most
> designers already use something like the proposal suggests but with a custom
> {% include %} hierachy. Introducing a "special" syntax can only open up the
> possibilities for performance tweaking.
>
>>
>> 2. Verbosity: There's a lot of tags (well, 4, but that's a lot to me)
>> wall-of-code stuff like
>> https://github.com/idangazit/formrendering/blob/master/djangocon_sketch.html#L62-83
>> doesn't particularly give me the warm fuzzies. I think part of the
>> problem is that all the tags are `form*` which makes for a bit of
>> "bork bork bork" there.
>>
>> I think it might be possible to simplify things somewhat here, so
>> here's my rough thoughts:
>>
>> * Keep {% form %} -- it's obvious.
>> * Rename {% formfield %} to {% field %} -- it won't conflict, and it's
>> (fairly) obvious we're talking about a *form* field since we'll
>> usually be saying {% field myform.whatever %}.
>> * Drop {% formrow %} entirely. Instead, have {% field %} generate the
>> whole thing you're calling a "row".
>
> This was Idan's main point in iterating over my earlier proposal. He saw it as
> a very basic usecase that fields aren't necessary one per row. He had some
> excellent examples of fields, like firstname surname, that should fit into one
> row. I think we should be able to make these rowdesigns reusable.
>
> I for myself had a few situations in which I really wanted to have the feature
> while implementing designs a client gave me.
>
>> * Add {% widget %} which rendered just the field (i.e. what {%
>> formfield %} does now).
>> * Keep {% formconfig %}.
>>
>> This is verging dangerously close to bikeshedding, so the syntax
>> either way won't change my vote much.
>>
>> Thanks!
>>
>> Jacob
>
> Thanks for your input Jacob!
>
> --
> Servus,
> Gregor Müllegger
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
