From effcd80471c2ccbe9250532421d445531b041db9 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 7 Sep 2018 05:55:56 +0100 Subject: [PATCH] compactlinks: Optimise performance of DOM logic The createCompactList() function runs synchronously during the module execution burst. Due to it visually changing the page, I won't defer it with rIC for the time being, although that should be considered for the future. For this commit, I'm trying to make it fit the budget of <50ms because ULS is currently usually taking 80ms-180ms on desktop (MacBook/Chrome CPU/4), and that's during batch execution with other modules as well, thus freezing the UI thread for much longer than that. constructor: * Remove needless clone of jQuery object. Use $foo instead of $( $foo ). * Remove creation of 'interlanguageList' and 'compactList' objects that are immediately removed and re-created by init(). init/getInterlanguageList: * Use the HTMLElement.lang and HTMLAnchorElement.href properties directly instead of the DOM getAttribute(). This means stores a full url instead of a relative url, which should help avoid other bugs in the future. * Remove needless jQuery() constructor and jQuery.text() call. Use Node.textContent directly instead. * Use HTMLElement#querySelectorAll instead of jQuery#find(). init/getCompactList/../filterByLangsInText: * Avoid jQuery() constructor and jQuery.attr(), use the HTMLElement.lang property directly. * Avoid jQuery() selector, call querySelectorAll directly. init/getCompactList/../getCommonLanguages/../getFrequentLanguageList: * Avoid temporary array copies from concat() and function overhead with forEach() and filter(). Instead, keep only a single array, and iterate it once. init/getCompactList/../filterByBadges (~10m -> ~0.5ms): * Use one query via $(), instead of two queries $()+find(). * Use $.map() directly instead of map()+fakejQueryObject+toArray(). * Use querySelector(One) for the child instead of $()+find(). * Use HTMLElement.lang property directly. init/hideOriginal (~5m -> ~0.8ms): * Use querySelectorAll() directly instead of jQuery find(). * Set HTMLElement.style directly instead of jQuery() css(). init/render/addTrigger: * Use createElement() and direct properties instead of $(), addClass(), prop() and text(). * The mw.msg() calls use text() and jqueryMsg#parser which is expensive. Use plain() for 'ext-uls-compact-link-info', which doesn't need parsing. Keep text() for the other message, and document why. init/listen: * Use async Deferred#then() instead of sometimes-sync Deferred#done(). Bug: T127328 Change-Id: I424c34fb82c8e95407f7b934e6d42019becbf909 --- resources/js/ext.uls.common.js | 43 +++++---- resources/js/ext.uls.compactlinks.js | 126 +++++++++++++++++---------- 2 files changed, 101 insertions(+), 68 deletions(-) diff --git a/resources/js/ext.uls.common.js b/resources/js/ext.uls.common.js index d70bff35..3b1a6e37 100644 --- a/resources/js/ext.uls.common.js +++ b/resources/js/ext.uls.common.js @@ -180,35 +180,34 @@ * @return {Array} List of language codes without duplicates. */ mw.uls.getFrequentLanguageList = function ( countryCode ) { - var unique = [], - list = [ - mw.config.get( 'wgUserLanguage' ), - mw.config.get( 'wgContentLanguage' ), - mw.uls.getBrowserLanguage() - ] - .concat( mw.uls.getPreviousLanguages() ) - .concat( mw.uls.getAcceptLanguageList() ); + var i, j, lang, + ret = [], + lists = [ + [ + mw.config.get( 'wgUserLanguage' ), + mw.config.get( 'wgContentLanguage' ), + mw.uls.getBrowserLanguage() + ], + mw.uls.getPreviousLanguages(), + mw.uls.getAcceptLanguageList() + ]; countryCode = countryCode || mw.uls.getCountryCode(); - if ( countryCode ) { - list = list.concat( $.uls.data.getLanguagesInTerritory( countryCode ) ); + lists.push( $.uls.data.getLanguagesInTerritory( countryCode ) ); } - list.forEach( function ( lang ) { - if ( unique.indexOf( lang ) === -1 ) { - unique.push( lang ); + for ( i = 0; i < lists.length; i++ ) { + for ( j = 0; j < lists[ i ].length; j++ ) { + lang = lists[ i ][ j ]; + // Make flat, make unique, and ignore unknown/unsupported languages + if ( ret.indexOf( lang ) === -1 && $.uls.data.getAutonym( lang ) !== lang ) { + ret.push( lang ); + } } - } ); + } - // Filter out unknown and unsupported languages - unique = unique.filter( function ( langCode ) { - // If the language is already known and defined, just use it. - // $.uls.data.getAutonym will resolve redirects if any. - return $.uls.data.getAutonym( langCode ) !== langCode; - } ); - - return unique; + return ret; }; }() ); diff --git a/resources/js/ext.uls.compactlinks.js b/resources/js/ext.uls.compactlinks.js index f6a6182b..edc81a95 100644 --- a/resources/js/ext.uls.compactlinks.js +++ b/resources/js/ext.uls.compactlinks.js @@ -72,7 +72,7 @@ * @return {string[]} List of language codes supported by the article */ function filterByBabelLanguages( languages ) { - var babelLanguages = mw.config.get( 'wgULSBabelLanguages', [] ); + var babelLanguages = mw.config.get( 'wgULSBabelLanguages' ) || []; return babelLanguages.filter( function ( language ) { return languages.indexOf( language ) >= 0; @@ -86,8 +86,7 @@ * @return {string[]} List of language codes supported by the article */ function filterBySitePicks( languages ) { - var picks = mw.config.get( 'wgULSCompactLinksPrepend', [] ); - + var picks = mw.config.get( 'wgULSCompactLinksPrepend' ) || []; return picks.filter( function ( language ) { return languages.indexOf( language ) >= 0; } ); @@ -136,27 +135,37 @@ */ function filterByAssistantLanguages( languages ) { var assistantLanguages = mw.user.options.get( 'translate-editlangs' ); - - if ( assistantLanguages && assistantLanguages !== 'default' ) { - return assistantLanguages.split( /,\s*/ ).filter( function ( language ) { - return languages.indexOf( language ) >= 0; - } ); + if ( !assistantLanguages || assistantLanguages === 'default' ) { + return []; } - return []; + return assistantLanguages.split( /,\s*/ ).filter( function ( language ) { + return languages.indexOf( language ) >= 0; + } ); } /** * @class * @constructor - * @param {string|jQuery} interlanguageList Selector for interlanguage list + * @param {HTMLElement} listElement Interlanguage list element * @param {Object} options */ - function CompactInterlanguageList( interlanguageList, options ) { - this.$interlanguageList = $( interlanguageList ); + function CompactInterlanguageList( listElement, options ) { + this.listElement = listElement; this.options = options || {}; - this.interlanguageList = {}; - this.compactList = {}; + + /** + * @private + * @property {Object} interlanguageList + */ + this.interlanguageList = null; + + /** + * @private + * @property {Object} interlanguageList + */ + this.compactList = null; + this.commonInterlanguageList = null; this.$trigger = null; this.compactSize = 0; @@ -176,7 +185,6 @@ if ( this.listSize <= max ) { // Not enough languages to compact the list mw.hook( 'mw.uls.compactlinks.initialized' ).fire( false ); - return; } @@ -311,7 +319,7 @@ this.$trigger.one( 'click', function () { // Load the ULS now. - mw.loader.using( 'ext.uls.mediawiki' ).done( function () { + mw.loader.using( 'ext.uls.mediawiki' ).then( function () { self.createSelector( self.$trigger ); self.$trigger.click(); } ); @@ -324,11 +332,10 @@ * @return {Object} */ CompactInterlanguageList.prototype.getCompactList = function () { - var language, languages, compactLanguages, i, - compactedList = {}; + var language, languages, compactLanguages, i, compactedList; + compactedList = {}; languages = Object.keys( this.interlanguageList ); - compactLanguages = this.compact( languages ); for ( i = 0; i < compactLanguages.length; i++ ) { @@ -413,9 +420,8 @@ */ CompactInterlanguageList.prototype.filterByLangsInText = function ( languages ) { var languagesInText = []; - - $( '#mw-content-text [lang]' ).each( function ( i, el ) { - var lang = convertMediaWikiLanguageCodeToULS( $( el ).attr( 'lang' ) ); + $.each( document.querySelectorAll( '#mw-content-text [lang]' ), function ( i, el ) { + var lang = convertMediaWikiLanguageCodeToULS( el.lang ); if ( languagesInText.indexOf( lang ) === -1 && languages.indexOf( lang ) >= 0 ) { languagesInText.push( lang ); } @@ -435,11 +441,14 @@ * @return {Array} List of language codes in which there are articles with badges */ CompactInterlanguageList.prototype.filterByBadges = function () { - return $( '#p-lang' ).find( '[class*="badge"]' ).map( function ( i, el ) { - return convertMediaWikiLanguageCodeToULS( - $( el ).find( '.interlanguage-link-target' ).attr( 'lang' ) - ); - } ).toArray(); + return $.map( + document.querySelectorAll( '#p-lang [class*="badge"]' ), + function ( el ) { + return convertMediaWikiLanguageCodeToULS( + el.querySelector( '.interlanguage-link-target' ).lang + ); + } + ); }; /** @@ -451,13 +460,12 @@ CompactInterlanguageList.prototype.getInterlanguageList = function () { var interlanguageList = {}; - this.$interlanguageList.find( '.interlanguage-link-target' ).each( function () { - var langCode = convertMediaWikiLanguageCodeToULS( this.getAttribute( 'lang' ) ); - + $.each( this.listElement.querySelectorAll( '.interlanguage-link-target' ), function ( i, el ) { + var langCode = convertMediaWikiLanguageCodeToULS( el.lang ); interlanguageList[ langCode ] = { - href: this.getAttribute( 'href' ), - autonym: $( this ).text(), - element: this + href: el.href, + autonym: el.textContent, + element: el }; } ); @@ -486,29 +494,55 @@ * Hide the original interlanguage list */ CompactInterlanguageList.prototype.hideOriginal = function () { - this.$interlanguageList.find( '.interlanguage-link' ).css( 'display', 'none' ); + var links = this.listElement.querySelectorAll( '.interlanguage-link' ), + i = links.length; + while ( i-- ) { + links[ i ].style.display = 'none'; + } }; /** * Add the trigger at the bottom of the language list */ CompactInterlanguageList.prototype.addTrigger = function () { - var $trigger; + var trigger = document.createElement( 'button' ); + trigger.className = 'mw-interlanguage-selector mw-ui-button'; + trigger.title = mw.message( 'ext-uls-compact-link-info' ).plain(); + // Use text() because the message needs {{PLURAL:}} + trigger.textContent = mw.message( + 'ext-uls-compact-link-count', + mw.language.convertNumber( this.listSize - this.compactSize ) + ).text(); - $trigger = $( '