Skip to content

Fix(Perf): remove per-row SQL queries from voucher views and the init activation check - #280

Open
jdurand-teclib wants to merge 1 commit into
11.0/bugfixesfrom
perf_optimization
Open

jdurand-teclib wants to merge 1 commit into
11.0/bugfixesfrom
perf_optimization

Conversation

@jdurand-teclib

Copy link
Copy Markdown
  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

The fix removes the queries that the Credit plugin ran once per row in its three list views. The consumed details of a busy voucher drop from 17,532 to 262 SQL queries, and its server time from 978 ms to 251 ms. Each GLPI request also runs 2 fewer queries. The rendered output does not change: the HTML of the ticket tab, the entity tab and the consumed details is identical between the two versions, once random ids and CSRF tokens are masked.
Three configurations were tested on the same instance, with the same data: plugin disabled (core cost only, for the core pages), before (main branch, 1.15.8) and after (fix).

  • Instance: GLPI 11.0 (11.0/bugfixes), e2e environment, Credit plugin 1.15.8, with and without the fix. Debug mode and Xdebug off.
  • Data: 2,000 tickets, 500 vouchers (100 in the root entity, 60 active), 50,000 consumptions, 500 ticket configs, 18 entity configs. The distribution is uneven on purpose: 20 tickets (1%) carry half of the consumptions (ticket 1 has 1,263) and 5 vouchers carry the other half (voucher 1 has 5,833).
  • Measured scenarios: the core home page, the main tab of ticket 1, the "Credit vouchers" tab of ticket 1, the "Credit vouchers" tab of the root entity, the consumed details of voucher 1, the voucher list, the voucher form, a ticket created, updated and purged, and a followup that consumes a voucher.
  • Measurement method: Chromium driven by Playwright, 1 warm-up and 5 measured runs per scenario. Times come from the browser Performance API. A timing gain counts only when the min–max ranges of before and after do not overlap.
  • SQL: the MariaDB slow log at a zero threshold, in a separate pass with one run per scenario. The count includes every query of the scenario's HTTP requests.

SQL Queries

The three plugin views no longer depend on the number of rows: the remaining queries are the GLPI page itself, plus one user lookup per distinct user.

Queries Without plugin Before After Difference
Consumed details, voucher with 5,833 consumptions n/a 17,532 262 -17,270 (99%)
Ticket "Credit vouchers" tab, 1,263 consumptions n/a 2,906 248 -2,658 (91%)
Entity "Credit vouchers" tab, 100 vouchers n/a 558 148 -410 (73%)
glpi_plugins queries per HTTP request 11 15 13 -2 (13%)
Ticket main tab 343 380 367 -13 (3%)
Ticket create, update and purge (48 requests) 2,975 3,186 3,055 -131 (4%)
Followup consuming a voucher 802 883 853 -30 (3%)

Before the fix, displayConsumed() loaded the ticket, its category and its user for each consumption: 3 queries per row. It now reads tickets and categories with the consumptions in one LEFT JOIN query, and builds each user link once per user.
showForTicket() read the voucher and the user of each row. Then it called PluginCreditEntity::showForItemtype($entity, 'Ticket'), which computed every active voucher of the entity (link, type, two consumption sums and an iframe modal each) and displayed nothing, because the template is only rendered for an Entity. The call is removed, and voucher names are loaded in one IN query.
showForItemtype() re-read each voucher with getById() although the row was already fetched, read its type, and summed its consumptions twice. It now reuses the row, caches the type links, and gets all the sums from one GROUP BY query (getConsumedForCredits()).
plugin_init_credit() checked $plugin->isActivated('credit'). During init this calls getFromDBbyDir(), which costs 2 queries on every request. The core only initializes active plugins, so the check is removed. The 2 remaining extra glpi_plugins queries per request belong to the core's own load of an active plugin.

Response Times

The consumed details and the ticket tab are clearly faster. The other changes stay within the noise of a local database.

Median (min–max) Before After Difference
Consumed details, server time (ms) 978 (887–2,542) 251 (245–281) -74%
Consumed details, fully loaded (ms) 2,034 (1,705–3,672) 1,080 (1,053–1,158) -47%
Ticket "Credit vouchers" tab, tab server time (ms) 204 (192–377) 80 (64–108) -61%
Ticket "Credit vouchers" tab, fully loaded (ms) 481 (431–657) 322 (295–353) -33%
Entity "Credit vouchers" tab, tab server time (ms) 100 (75–142) 72 (43–78) not significant
Ticket main tab, fully loaded (ms) 490 (464–496) 450 (410–479) not significant

The test database runs on the same machine, so a query costs about 0.1 to 0.5 ms. With a remote database, the gain depends on network latency: at 0.5 ms per round trip, the 17,270 queries saved on the consumed details would save about 8.6 s. This is an estimate, not a measurement.
Two browser costs are unchanged because they need an interface change: the ticket tab still renders all rows (4.3 MB of HTML for 1,263 consumptions), and the entity tab still builds one hidden iframe modal per voucher (1 MB for 100 vouchers).

Other changes

  • Purging a ticket now deletes its default voucher options (glpi_plugin_credit_ticketconfigs) through an item_purge hook. Before, each purge left an orphan row. Consumptions are kept, because they still count against their voucher.
  • The ticket tab counts consumptions only when tab counters are shown. When counters are off, the tab label is now always plural.
  • displayConsumed() no longer passes the comments option to ITILCategory::getName(). GLPI 11 ignores this option and raised an E_USER_WARNING on every row with a category.

This branch has not been deployed

No deployments
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.

1 participant