Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use django-table2 #1589

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use django-table2 #1589

wants to merge 1 commit into from

Conversation

rixx
Copy link
Member

@rixx rixx commented Sep 27, 2023

This PR attempts to include django-table2 in pretalx.

Status

The proposal list has been replaced with a feature-complete equivalent table using django-table-2, except that the changed code does not as yet respect pagination page size from urlparams. Queries have increased by a constant amount to permit links to submission types (which we didn't have previously).

Performance

Rendering tables with django-table2 seems to be noticeably slower than running through our previous giant template. More so in DEBUG mode (as templates aren't cached then, and we're using more templates), but even in DEBUG=False, there seems to be a significant penalty:

## debug=true
page size  old        django-tables2
25         550-700ms   800-950ms
100        750-950ms   900-1200ms


## debug=false
page size  old        django-tables2
25         250-330ms   300-350ms
100        350-470ms   580-700ms

I tried to attach py-spy, and can't spot anything obvious in the block belonging to django-table2 (svg attached).
2023-09-27T17:14:16+02:00

SVG behind the fold

profile

With a performance penalty like this, I'm against using django-table2 despite the obvious benefits in maintainable code, especially as larger pages seem to be hit disproportionately, and our review dashboard is an unpaginated page of all proposals. (Of course, once we have decent bulk editing, we can paginate or lazy-load there, it's not like we're out of solutions, but still).

I still want to see perf penalties on the big review dashboard, and then I'll decide what to do.

Todo

  • Decide if we want to use django-table2
  • Migrate all lists to django-table2
    • review dashboard
    • questions list
    • feedback list
    • allfeedback list
    • taglist
    • speakerlist
    • informationlist
    • eventhistory
    • submissiontypelist
    • tracklist
    • accesscodelist
  • Build generic filter form UI
  • Build table builder / column selection UI
  • Build exporter UI

@rixx
Copy link
Member Author

rixx commented Sep 27, 2023

There's of course the option to take the performance hit in return for easier maintenance, but I think due to the very low amount of features I'm likely to use, I'd rather look at some of the django-table2 code and build my own version (with much less bells and whistles, mostly some iterators etc), which could also integrate better with what pretalx needs – you'll see that I had to already do some pretty gnarly subclassing because django-table2 doesn't really believe in making defaults easy to override …

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant