Improve the performance of the script delivering the permissions search filter functionality in admin/people/permissions.

Created on 5 September 2024, 14 days ago
Updated 16 September 2024, 3 days ago

Problem/Motivation

Script user.permissions.js, located in /core/modules/user, is currently using jQuery to perform the operations of showing and hiding table rows that match (or not) the text being typed in real-time in the filter box. It turns out using jQuery has a negative performance impact. The end-user might experience some short 'chokes' when typing a search string, as the Javascript is performing searches across the list of table rows in realtime. Performance can be improved by simply removing jQuery from the equation and using plain simple 'vanilla' Javascript to show or hide table rows.

Also, there are some opportunities to improve the script structure to make it more performant.

Steps to reproduce

Enter the Permissions admin page in /admin/people/permissions and type a string that might find some occurrences, like 'Content' or 'Access'.

In order to test the performance enhancement I am mentioning on this issue, I suggest people add console.time('performanceTest') and console.timeEnd('performanceTest') into strategic portions of the code, both in the previous version (branch 11.x) and the new one (branch 3472217-improve-permission-search-filter). The opening console message (console.time) should be placed right after the opening of the filterPermissionList(e) function. The closing console message (console.timeEnd) should come as the last line of code within that same function.

One detail worth mentioning: the new code is interrupting code execution when the search string is equivalent to the previous search string. The use case for this is when users use arrow keys that count as a keystroke, that do not affect the search text; we don't need to waste time analyzing this particular situation, and the code will interrupt execution/analysis early in the cycle. But that line of code will cause the timer to throw a warning on the console because it won't be properly closed. When running performance tests, replace this line...

if (rawQuery === queryTransitional) return;

... with something like this:

if (rawQuery === queryTransitional) {
  console.timeEnd('performanceTest');
  return;
}

Proposed resolution

Remove any jQuery code within behavior tableFilterByText; establish a mechanism to improve the search performance as users type within the filter input field.

Remaining tasks

A proposed solution is already available for testing on the associated repository.

User interface changes

An improved user experience in terms of responsiveness when users are typing a search string to filter from the existing permissions.

Introduced terminology

--

API changes

--

Data model changes

--

Release notes snippet

--

πŸ“Œ Task
Status

Closed: duplicate

Version

11.0 πŸ”₯

Component
User systemΒ  β†’

Last updated about 13 hours ago

Created by

πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ

Live updates comments and jobs are added and updated live.
  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

  • jQuery

    Affects the version, handling, or usage of the jQuery javascript library.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mabho
  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ
  • Pipeline finished with Failed
    13 days ago
    Total: 128s
    #275271
  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ

    I am introducing some enhancements with the purpose of making the code run faster.

    First things first, removing jQuery delivers relevant performance enhancement. It is amazing how slow the code can get on some operations triggered by jQuery. This becomes more dramatic on a script being activate on each keystroke by the client.

    Apart from that, the code is also introducing transitional variables storing information on the most recent text typed and sample search results extracted. By comparing the previous search string with the current, the program can determine if the user is incrementally typing a word or phrase. If so, it will incrementally run the new row filtering from the sample results on the previous round, instead of doing it from scratch every time.

    This dramatically reduces the number of rows the program needs to analyze, thus improving performance.

    A merge request is already available.

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    I'm on my phone so I can't fix this but the tests are not running because there is a typo: beiong

  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ

    Real-time commenting and adjusting ;-) Just fixed the typo, many thanks for the quick feedback!

  • Pipeline finished with Success
    13 days ago
    Total: 493s
    #275278
  • Status changed to Needs review 13 days ago
  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ
  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ
  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ
  • πŸ‡«πŸ‡·France nod_ Lille

    for filtering I'd rather look into ✨ Create Javascript library for searching rendered lists on the client. Needs work rather than spend time on each individual filter script

  • πŸ‡ΊπŸ‡ΈUnited States nicxvan

    Can the change here be used as a base for the shared library?

  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ

    Thank you @_nod and @nicxvan. I can see that initiative is pretty advanced and removes the dependency on jQuery... Maybe the suggested approach here (taking advantage of a subset of the records being searched based on the text previously typed) can be ported into that script for improved performance... or not. I will take a look at it.

  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ

    My only concern about this permissions table is its mechanism to hide/show the parent of the found record (in this case, the module name associated with the found permission). I wonder if the implementation being carried out here (https://git.drupalcode.org/project/drupal/-/merge_requests/6309/diffs#58...) is taking care of this. This is speculation, though, and I need to look into that implementation.

  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ

    I moved my activity into issue thread ✨ Create Javascript library for searching rendered lists on the client. Needs work because it is proposing a single library to be used everywhere a filter is needed. It promotes code reuse and provides a pluggable mechanism that can more easily be reused.

  • Status changed to Closed: duplicate 3 days ago
  • πŸ‡§πŸ‡·Brazil mabho Rio de Janeiro, RJ
Production build 0.71.5 2024