From d0b4c2f16d34031bbf541414e5691959fc25af6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrkan=20G=C3=BCr?= Date: Thu, 9 Jul 2026 19:55:54 +0200 Subject: [PATCH] servershell: speed up result table rendering Selecting and working on many objects is slow because of the current per-row jQuery construction and per-cell linear scans. This commit tries to speed up via multiple changes. Feels much snappier but don't have a solid benchmark yet. Mostly work of Opus, still sending all the inputs through escape_html to avoid XSS. - Rewrite update_result/get_row_html to build the body as one HTML string (single innerHTML) instead of per-element jQuery construction. - Precompute O(1) lookups per redraw (object/attribute/deleted/changed) instead of linear scans per cell. - Snapshot the servershell Proxy properties used in the loop (each access dispatches events). - Event delegation for inline-edit and relation links instead of per-cell/per-link handlers. - set_selected O(rows) instead of O(selected * rows). - Fix delete to redraw once instead of once per selected object. --- .../static/js/servershell/command.js | 4 +- .../static/js/servershell/result.js | 496 ++++++++++-------- 2 files changed, 269 insertions(+), 231 deletions(-) diff --git a/serveradmin/servershell/static/js/servershell/command.js b/serveradmin/servershell/static/js/servershell/command.js index 8f239d9f..5d633994 100644 --- a/serveradmin/servershell/static/js/servershell/command.js +++ b/serveradmin/servershell/static/js/servershell/command.js @@ -445,9 +445,11 @@ servershell.commands = { // Avoid duplicate deletion ... if (!servershell.to_commit.deleted.includes(object_id)) { servershell.to_commit.deleted.push(object_id); - servershell.update_result(); } }); + + // Redraw once after marking all selected objects + servershell.update_result(); }, setattr: function(attribute_value_string) { if (!validate_selected()) { diff --git a/serveradmin/servershell/static/js/servershell/result.js b/serveradmin/servershell/static/js/servershell/result.js index 1551dd40..217c1b3b 100644 --- a/serveradmin/servershell/static/js/servershell/result.js +++ b/serveradmin/servershell/static/js/servershell/result.js @@ -13,6 +13,13 @@ servershell.update_result = function() { // Memorize currently selected objects to restore let selected = servershell.get_selected(); + // servershell is a Proxy (servershell.js): reading a configured property + // fires jQuery events and allocates a wrapper on each access. Snapshot the + // ones used per row into plain values so the render loop stays cheap. + let shown_attributes = [...servershell.shown_attributes]; + let changes = servershell.to_commit.changes; + let offset = servershell.offset; + // Recreate table header let header = table.find('thead tr'); header.empty(); @@ -22,7 +29,7 @@ servershell.update_result = function() { ) ); header.append('#'); - servershell.shown_attributes.forEach((attribute, index) => header.append( + shown_attributes.forEach((attribute, index) => header.append( $('').append( $('').text(attribute), $(``).append( @@ -40,11 +47,32 @@ servershell.update_result = function() { ) )); - // Recreate table body + // O(1) lookups for the per-cell render loop below, built once. Keeps cell + // rendering from scanning objects / the attribute catalog / pending changes + // linearly for every cell (which makes a redraw quadratic in row count). + let ctx = { + deleted: new Set(servershell.to_commit.deleted), + changed_ids: new Set( + Object.keys(servershell.to_commit.changes).map(Number)), + pinned: new Set(servershell.pinned ?? []), + attribute_map: new Map( + servershell.attributes.map(a => [a.attribute_id, a])), + editable_sets: new Map( + Object.entries(servershell.editable_attributes).map( + ([servertype, attrs]) => [servertype, new Set(attrs)])), + // Proxied properties snapshotted above. + shown_attributes: shown_attributes, + changes: changes, + offset: offset, + }; + + // Build all rows as HTML strings and write the body with a single + // innerHTML assignment. One parser for the whole body is cheaper than + // per-element jQuery construction when there are many rows processed. let body = table.find('tbody'); - body.empty(); _lastCheckedIndex = null; - servershell.servers.forEach((object, index) => body.append(get_row_html(object, index + 1))); + let rows = servershell.servers.map((object, index) => get_row_html(object, index + 1, ctx)); + body[0].innerHTML = rows.join(''); // Restore previous selected objects servershell.set_selected(selected); @@ -60,117 +88,144 @@ servershell.update_result = function() { } }; +/** + * Escape a value for safe interpolation into an HTML string. + * + * get_row_html builds rows as HTML strings, so every dynamic value (attribute + * values, hostnames, ids) must be escaped through here before interpolation to + * avoid XSS. + * + * @param value + * @returns {string} + */ +escape_html = function(value) { + return String(value) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +}; + +/** + * Build the inner HTML for a relation/reverse attribute cell. + * + * Related objects are rendered as ctrl-clickable spans (a single delegated + * click handler on the table, see $(document).ready, does the inspecting). + * Returns an HTML string; hostnames are escaped. + * + * @param value hostname string (multi=false) or array of hostnames (multi=true) + * @returns {string} + */ +relation_links_html = function(value) { + if (value === null || value === undefined) { + return ''; + } + + // Normalize single relations and multi relations to a sorted list so the + // display matches get_string() ordering. + let hostnames = Array.isArray(value) ? value.slice().sort() : [value]; + + return hostnames.map(function(hostname) { + let safe = escape_html(hostname); + return `${safe}`; + }).join(', '); +}; + /** * Get HTML for table row * - * Returns the HTML for the result table body row based on the object and row - * number. + * Returns the HTML string for a result table body row based on the object and + * row number. Built as a string (not jQuery elements) so the whole body can be + * assigned with a single innerHTML write in update_result. * * @param object * @param number - * @returns {jQuery|HTMLElement} + * @param ctx precomputed lookups from update_result (see there) + * @returns {string} */ -get_row_html = function(object, number) { - let row = $(''); - row.data('oid', object.object_id); +get_row_html = function(object, number, ctx) { + let classes = []; // Mark row as deleted. Make sure this wins over other colors // such as the state otherwise the user will not see objects marked for // deletion. - if (servershell.to_commit.deleted.includes(object.object_id)) { - row.addClass('delete'); + if (ctx.deleted.has(object.object_id)) { + classes.push('delete'); } - if (object.hasOwnProperty('state')) { - row.addClass(`state-${object.state}`); + classes.push(`state-${object.state}`); } - - if (Object.keys(servershell.to_commit.changes).map(value => Number.parseInt(value)).includes(object.object_id)) { - row.addClass('highlight'); + if (ctx.changed_ids.has(object.object_id)) { + classes.push('highlight'); } - - if ((servershell.pinned ?? []).includes(object.object_id)) { - row.addClass('highlight'); + if (ctx.pinned.has(object.object_id)) { + classes.push('highlight'); } - // Standard columns which should always be present - row.append($('').append($('').val(object.object_id))); - row.append($('').text(number + servershell.offset)); + let class_attr = classes.length ? ` class="${escape_html(classes.join(' '))}"` : ''; + // data-oid lets the delegated inline-edit handler recover the object id. + let html = ``; - let changes = servershell.to_commit.changes; - servershell.shown_attributes.forEach(function(attribute_id) { - if (is_editable(object.object_id, attribute_id)) { - let cell = $(''); - cell.data('aid', attribute_id); - - // Some helper variables - let object_id = object.object_id; - let attribute = servershell.get_attribute(attribute_id); - - // Changes in to_commit we have to display - let change; - if (object_id in changes && attribute_id in changes[object_id]) { - change = changes[object_id][attribute_id]; - } - - if (change) { - if (attribute.multi) { - let to_add = change.add.join(', '); - let to_delete = change.remove; - let value = object[attribute_id].filter(v => !to_delete.includes(v)).join(', '); - - cell.text(value); - let del = $(''); - del.text(to_delete.join(', ')); - if (value.length > 0) { - cell.append(', '); - } - cell.append(del); - - let add = $(''); - add.text(to_add); - cell.append(add); - } - else { - let to_delete = change.old === null ? '' : change.old; - let new_value = change.new === undefined ? '': change.new; + // Standard columns which should always be present + html += ``; + html += `${escape_html(number + ctx.offset)}`; + + let changes = ctx.changes; + ctx.shown_attributes.forEach(function(attribute_id) { + // Looked up once from the precomputed map instead of scanning the + // full attribute catalog per cell. + let attribute = ctx.attribute_map.get(attribute_id); + let editable = is_editable(object, attribute_id, ctx.editable_sets); + + // Changes in to_commit we have to display (only on editable cells) + let change; + if (editable && object.object_id in changes && attribute_id in changes[object.object_id]) { + change = changes[object.object_id][attribute_id]; + } - let del = $(''); - del.text(to_delete); - cell.append(del); + let inner; + if (change) { + if (attribute.multi) { + let to_add = change.add.join(', '); + let to_delete = change.remove; + let value = object[attribute_id].filter(v => !to_delete.includes(v)).join(', '); - let current = $(''); - current.text(new_value); - cell.append(current); + inner = escape_html(value); + if (value.length > 0) { + inner += ', '; } - } - else if (attribute.type === 'relation' || attribute.type === 'reverse') { - // Render related objects as ctrl-clickable links to inspect - append_relation_links(cell, object[attribute_id]); + inner += `${escape_html(to_delete.join(', '))}`; + inner += `${escape_html(to_add)}`; } else { - cell.text(get_string(object_id, attribute_id)); + let to_delete = change.old === null ? '' : change.old; + let new_value = change.new === undefined ? '' : change.new; + + inner = `${escape_html(to_delete)}${escape_html(new_value)}`; } + } + else if (attribute.type === 'relation' || attribute.type === 'reverse') { + // Reverse attributes are readonly and land here too; both editable + // and readonly relations render as ctrl-clickable inspect links. + inner = relation_links_html(object[attribute_id]); + } + else { + inner = escape_html(get_string(object, attribute_id, attribute)); + } - register_inline_editing(cell); - row.append(cell); + if (editable) { + // 'editable' class + data-aid let a single delegated dblclick + // handler on the table target these cells (see $(document).ready). + html += `${inner}`; } else { - let attribute = servershell.get_attribute(attribute_id); - let cell = $(''); - if (attribute.type === 'relation' || attribute.type === 'reverse') { - // Reverse attributes are readonly and land here, make them - // ctrl-clickable links to inspect the related objects. - append_relation_links(cell, object[attribute_id]); - } - else { - cell.text(get_string(object.object_id, attribute_id)); - } - row.append(cell); + html += `${inner}`; } }); - return row; + html += ''; + return html; }; /** @@ -194,38 +249,45 @@ servershell.get_selected = function() { * @param object_ids */ servershell.set_selected = function(object_ids) { - let checkboxes = $('#result_table input[name=server]'); - object_ids.forEach(function(object_id) { - let checkbox = checkboxes.filter(`input[value=${object_id}]`); - if (checkbox.length) { - checkbox[0].checked = true; - } + // Single pass with Set membership, so restoring selection is O(rows) rather + // than O(selected * rows). Runs right after a full body rebuild, so + // unchecking the non-matches here is harmless. + let wanted = new Set(object_ids); + $('#result_table input[name=server]').each(function() { + this.checked = wanted.has(parseInt(this.value)); }); }; /** * Check if attribute is editable * - * @param object_id + * Takes the object directly (not the id) and a precomputed Map of + * servertype -> Set(editable attribute ids) to avoid per-cell linear scans. + * + * @param object * @param attribute_id + * @param editable_sets Map> * @returns boolean */ -is_editable = function(object_id, attribute_id) { - let object = servershell.get_object(object_id); - return attribute_id in object && servershell.editable_attributes[object.servertype].includes(attribute_id); +is_editable = function(object, attribute_id, editable_sets) { + let set = editable_sets.get(object.servertype); + return attribute_id in object && set !== undefined && set.has(attribute_id); }; /** * Get attribute value as string * - * @param object_id + * Takes the object and its already-resolved attribute directly to avoid the + * get_object / get_attribute linear scans on the hot render path. + * + * @param object * @param attribute_id + * @param attribute resolved attribute object * @returns {*} */ -get_string = function(object_id, attribute_id) { - let object = servershell.get_object(object_id); +get_string = function(object, attribute_id, attribute) { if (attribute_id in object && object[attribute_id] !== null) { - if (servershell.get_attribute(attribute_id).multi) { + if (attribute.multi) { return object[attribute_id].sort().join(', '); } else { return object[attribute_id].toString(); @@ -236,162 +298,119 @@ get_string = function(object_id, attribute_id) { }; /** - * Append relation/reverse attribute values as ctrl-clickable links + * Make row editable by double click * - * Each related object is identified by its hostname. Ctrl-click (or cmd-click - * on macOS) opens the inspect page for that hostname in a new tab. Plain click - * is left untouched so row selection and inline editing (double click) on - * editable relations keep working as before. + * Inline edit handler, bound once via delegation on the result table (see + * $(document).ready) so no per-cell handlers are registered. Reads the object + * and attribute from the double-clicked cell/row. * - * @param cell jQuery td element - * @param value hostname string (multi=false) or array of hostnames (multi=true) + * @param event dblclick event on a td.editable cell */ -append_relation_links = function(cell, value) { - if (value === null || value === undefined) { +handle_inline_edit = function (event) { + // Do not open another inline edit unless previous is finished + let previous = $('#inline-edit-save'); + if (previous.length) { return; } - // Normalize single relations and multi relations to a sorted list so the - // display matches get_string() ordering. - let hostnames = Array.isArray(value) ? value.slice().sort() : [value]; + let cell = $(event.target).closest('td'); + let row = cell.parent(); + let object_id = row.data('oid'); + let object = servershell.get_object(object_id); + let attribute_id = cell.data('aid'); + let attribute = servershell.get_attribute(attribute_id); - hostnames.forEach(function(hostname, index) { - if (index > 0) { - cell.append(', '); - } + // Select row for convenience + if (!servershell.get_selected().includes(object_id)) { + row.children('td:first').children('input').click(); + } - let link = $('').text(hostname); - link.click(function(event) { - // Only hijack ctrl/cmd-click, leave plain clicks alone. - if (!event.ctrlKey && !event.metaKey) { - return; + let current_value; + let changes = servershell.to_commit.changes; + if (object_id in changes && attribute_id in changes[object_id]) { + if (attribute.multi) { + current_value = object[attribute_id].filter(v => !changes[object_id][attribute_id].remove.includes(v)); + current_value = current_value.concat(changes[object_id][attribute_id].add); + } else { + if (changes[object_id][attribute_id].action === 'delete') { + current_value = ''; + } else { + current_value = changes[object_id][attribute_id].new; } - event.preventDefault(); - window.open( - `${servershell.urls.inspect}?hostname=${encodeURIComponent(hostname)}`, - '_blank' - ); - }); - cell.append(link); - }); -}; - -/** - * Make row editable by double click - * - * This will add the inline edit functionality which allows us to double - * click a cell manipulate it in a input or textarea and save it. This - * is the same as the multiadd, multidel, setattr commands. - * - * @param cell jQuery td element - */ -register_inline_editing = function(cell) { - cell.dblclick(function (event) { - // Do not open another inline edit unless previous is finished - let previous = $('#inline-edit-save'); - if (previous.length) { - return; } + } else { + current_value = servershell.get_object(object_id)[attribute_id]; + } - let cell = $(event.target).closest('td'); - let row = cell.parent(); - let object_id = row.data('oid'); - let object = servershell.get_object(object_id); - let attribute_id = cell.data('aid'); - let attribute = servershell.get_attribute(attribute_id); + let content; + if (attribute.multi) { + content = $('