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

Improve performance of the permissions page #6312

Open
klonos opened this issue Nov 30, 2023 · 4 comments
Open

Improve performance of the permissions page #6312

klonos opened this issue Nov 30, 2023 · 4 comments

Comments

@klonos
Copy link
Member

klonos commented Nov 30, 2023

https://www.drupal.org/project/drupal/issues/1203766 was raised back in 2011. As part of that issue, a performance improvement was committed to D7 (~30% decrease in time to load, with around 1,200 permissions checkboxes on the page.), and we have inherited that: https://git.drupalcode.org/project/drupal/-/commit/18cb27724c693967fe10d4ef348ee048084b3831

The discussions have continued on that issue for years to come, until it was eventually closed in October 2021, and a follow-up issue created: https://www.drupal.org/project/drupal/issues/3244564

There was a commit for this issue in Jul 2011 to both Drupal 7 and Drupal 8 (#24).

It was reopened shortly after to evaluate an alternative approach. In #38 and #39 it seems that new approach is 17% improvement. And supported in #44. So perhaps the way forward here is to open a new issue to evaluate the latest patch.

The new issue contains a patch that was initially proposed in the original issue, but it is not marked for backport to D7 (although the code changes seem to apply to D7, as well as to Backdrop). The comment when the patch was initially proposed as a follow-up states:

... since the committed code is quite a major workaround for performance, it would be a good idea to investigate, benchmark and compare this alternative approach.

Opening this issue here in order to:

  • try the patch that was originally proposed and see if it actually provides further performance gain (we'll need to benchmark the permissions page load time with vs w/o it)
  • track the new issue and see if there's any further progress/changes that can be backported
@yorkshire-pudding
Copy link
Member

@klonos - you might want to do something about the links to backdrop issues as these don't look relevant; it seems to have picked up the comment numbers from the D issue queue quote and turned them into links to backdrop issues.

@klonos
Copy link
Member Author

klonos commented Nov 30, 2023

Thanks for pointing that out @yorkshire-pudding ...fixed 👍🏼

@yorkshire-pudding
Copy link
Member

yorkshire-pudding commented Nov 30, 2023

Thanks for pointing that out @yorkshire-pudding ...fixed 👍🏼

Looks like one left to #24

@klonos
Copy link
Member Author

klonos commented Nov 30, 2023

Looks like one left to #24

That one has a legitimate link to d.o 😉

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

No branches or pull requests

2 participants