[MediaWiki-commits] [Gerrit] mediawiki/core[master]: resourceloader: Make cache-eval in mw.loader.work asynchronous
jenkins-bot has submitted this change and it was merged.
Change subject: resourceloader: Make cache-eval in mw.loader.work asynchronous
..
resourceloader: Make cache-eval in mw.loader.work asynchronous
This is an amended version of reverted commit 482ad8d9fb2e.
The difference is that this change is smaller.
We keep the eval as one large chunk. Not split up in <=50ms chunks and
spread out based on how fast code executes on the device. This means we keep
the issue of an irresponsive UI thread during execution, and trade it for
finishing quicker. This doesn't feel right, but is something we should change
separately, later because it has side-effects (T146510).
Doing it in one chunk is the status quo as we've had it for the last two
years. This commit merely defers the big eval chunk to one tick later so
that higher priority work may happen first (e.g. rendering).
Bug: T142129
Change-Id: I2b31b27554cd29b48d986ed6a6f698a77e59c0e5
---
M resources/src/mediawiki/mediawiki.js
1 file changed, 33 insertions(+), 16 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/src/mediawiki/mediawiki.js
b/resources/src/mediawiki/mediawiki.js
index f878e42..6b23439 100644
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1681,6 +1681,26 @@
}
/**
+* @private
+* @param {string[]} implementations Array containing
pieces of JavaScript code in the
+* form of calls to mw.loader#implement().
+* @param {Function} cb Callback in case of failure
+* @param {Error} cb.err
+*/
+ function asyncEval( implementations, cb ) {
+ if ( !implementations.length ) {
+ return;
+ }
+ mw.requestIdleCallback( function () {
+ try {
+ $.globalEval(
implementations.join( ';' ) );
+ } catch ( err ) {
+ cb( err );
+ }
+ } );
+ }
+
+ /**
* Make a versioned key for a specific module.
*
* @private
@@ -1733,7 +1753,7 @@
* @protected
*/
work: function () {
- var q, batch, concatSource, origBatch;
+ var q, batch, implementations,
sourceModules;
batch = [];
@@ -1763,19 +1783,18 @@
mw.loader.store.init();
if ( mw.loader.store.enabled ) {
- concatSource = [];
- origBatch = batch;
+ implementations = [];
+ sourceModules = [];
batch = $.grep( batch, function
( module ) {
- var source =
mw.loader.store.get( module );
- if ( source ) {
-
concatSource.push( source );
+ var implementation =
mw.loader.store.get( module );
+ if ( implementation ) {
+
implementations.push( implementation );
+
sourceModules.push( module );
return false;
}
return true;
} );
- try {
- $.globalEval(
concatSource.join( ';' ) );
- } catch ( err ) {
+ asyncEval( implementations,
function ( err ) {
// Not good, the cached
mw.loader.implement calls failed! This should
// never happen,
barring R
[MediaWiki-commits] [Gerrit] mediawiki/core[master]: resourceloader: Make cache-eval in mw.loader.work asynchronous
Krinkle has uploaded a new change for review.
https://gerrit.wikimedia.org/r/316395
Change subject: resourceloader: Make cache-eval in mw.loader.work asynchronous
..
resourceloader: Make cache-eval in mw.loader.work asynchronous
This is an amended version of reverted commit 482ad8d9fb2e.
The difference is that this change is smaller.
We keep the eval as one large chunk. Not split up in <=50ms chunks and
spread out based on how fast code executes on the device. This means we keep
the issue of an irresponsive UI thread during execution, and trade it for
finishing quicker. This doesn't feel right, but is something we should change
separately, later because it has side-effects (T146510).
Doing it in one chunk is the status quo as we've had it for the last two
years. This commit merely defers the big eval chunk to one tick later so
that higher priority work may happen first (e.g. rendering).
Bug: T142129
Change-Id: I2b31b27554cd29b48d986ed6a6f698a77e59c0e5
---
M resources/src/mediawiki/mediawiki.js
1 file changed, 33 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/95/316395/1
diff --git a/resources/src/mediawiki/mediawiki.js
b/resources/src/mediawiki/mediawiki.js
index f878e42..6b23439 100644
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1681,6 +1681,26 @@
}
/**
+* @private
+* @param {string[]} implementations Array containing
pieces of JavaScript code in the
+* form of calls to mw.loader#implement().
+* @param {Function} cb Callback in case of failure
+* @param {Error} cb.err
+*/
+ function asyncEval( implementations, cb ) {
+ if ( !implementations.length ) {
+ return;
+ }
+ mw.requestIdleCallback( function () {
+ try {
+ $.globalEval(
implementations.join( ';' ) );
+ } catch ( err ) {
+ cb( err );
+ }
+ } );
+ }
+
+ /**
* Make a versioned key for a specific module.
*
* @private
@@ -1733,7 +1753,7 @@
* @protected
*/
work: function () {
- var q, batch, concatSource, origBatch;
+ var q, batch, implementations,
sourceModules;
batch = [];
@@ -1763,19 +1783,18 @@
mw.loader.store.init();
if ( mw.loader.store.enabled ) {
- concatSource = [];
- origBatch = batch;
+ implementations = [];
+ sourceModules = [];
batch = $.grep( batch, function
( module ) {
- var source =
mw.loader.store.get( module );
- if ( source ) {
-
concatSource.push( source );
+ var implementation =
mw.loader.store.get( module );
+ if ( implementation ) {
+
implementations.push( implementation );
+
sourceModules.push( module );
return false;
}
return true;
} );
- try {
- $.globalEval(
concatSource.join( ';' ) );
- } catch ( err ) {
+ asyncEval( implementations,
function ( err ) {
// Not good, the cached
mw.loader.implement calls failed! This should
[MediaWiki-commits] [Gerrit] mediawiki/core[master]: resourceloader: Make cache-eval in mw.loader.work asynchronous
jenkins-bot has submitted this change and it was merged. Change subject: resourceloader: Make cache-eval in mw.loader.work asynchronous .. resourceloader: Make cache-eval in mw.loader.work asynchronous * Add 'if !batch.length' in work() to avoid needless initialisation of mw.loader.store(). work() can be called without anything in the batch because using() will call enqueue() for any non-ready modules in order to get a job callback. This will invoke work() as side-effect but will have no work to do. This was previously here but got moved in 488c5d9. * Add 'if !implementations.length' to avoid needless scheduling of an idle callback when none of the modules in the batch are found in local storage (e.g. first view). Bug: T142129 Change-Id: Icd39423aec35f25162d2443b1f0507f6f0c748a3 --- M resources/src/mediawiki/mediawiki.js 1 file changed, 53 insertions(+), 25 deletions(-) Approvals: Gilles: Looks good to me, approved Catrope: Looks good to me, but someone else must approve jenkins-bot: Verified diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js index 7ceb5fe..122f232 100644 --- a/resources/src/mediawiki/mediawiki.js +++ b/resources/src/mediawiki/mediawiki.js @@ -1195,7 +1195,7 @@ // Force jQuery behaviour to be for crossDomain. Otherwise jQuery would use // XHR for a same domain request instead of
