GWicke has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/66983


Change subject: Performance: Avoid API requests by reusing image expansions
......................................................................

Performance: Avoid API requests by reusing image expansions

Rendering an image involves an API request to retrieve image metadata (whether
it exists, dimensions etc). This is expensive for the API cluster.

This patch avoids the API load for most normal edits by reusing the expansions
for unmodified image expansions from a cached DOM similar to transclusion and
extension content.

Change-Id: I80139a8e7e4be6d34340f007b2a980b641a4c6eb
---
M js/lib/ext.core.LinkHandler.js
M js/lib/ext.core.TemplateHandler.js
M js/lib/mediawiki.DOMUtils.js
M js/lib/mediawiki.Util.js
M js/lib/mediawiki.parser.environment.js
5 files changed, 44 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid 
refs/changes/83/66983/1

diff --git a/js/lib/ext.core.LinkHandler.js b/js/lib/ext.core.LinkHandler.js
index 6ccfeb3..048ec84 100644
--- a/js/lib/ext.core.LinkHandler.js
+++ b/js/lib/ext.core.LinkHandler.js
@@ -14,7 +14,8 @@
        sanitizerLib = require( './ext.core.Sanitizer.js' ),
        Sanitizer = sanitizerLib.Sanitizer,
        SanitizerConstants = sanitizerLib.SanitizerConstants,
-       defines = require('./mediawiki.parser.defines.js');
+       defines = require('./mediawiki.parser.defines.js'),
+       DU = require('./mediawiki.DOMUtils.js').DOMUtils;
 // define some constructor shortcuts
 var KV = defines.KV,
     TagTk = defines.TagTk,
@@ -720,8 +721,11 @@
                        info = image.imageinfo[0];
                }
 
-               // We can roundtrip this now!
+               // We can roundtrip this now, but still need a good cache key. 
Use the
+               // source for now.
+               dataAttribs.cacheKey = dataAttribs.src;
                dataAttribs.src = undefined;
+
 
                // But we need some extra information!
                dataAttribs.img = {};
@@ -806,6 +810,7 @@
                }
 
                container.addAttribute( 'typeof', rdfaType );
+               container.addAttribute( 'about', env.newObjectId());
 
                var tokens = [
                                container,
@@ -833,6 +838,22 @@
                cb( { tokens: tokens } );
        }
 
+       // First check if we have a cached copy of this image expansion, and
+       // avoid any further processing if we have a cache hit.
+       var cachedImage = this.manager.env.imageCache[token.dataAttribs.src];
+       if (cachedImage) {
+               // Use the cached result.
+               // mw:DOMFragment wrapping is simplified as we know that we are
+               // dealing with a single subtree rooted either at a figure or a 
span.
+               var wrapperTokens = DU.getWrapperTokens(cachedImage.nodes),
+                       firstWrapperToken = wrapperTokens[0];
+               DU.addTypeOf(firstWrapperToken, 'mw:DOMFragment');
+               firstWrapperToken.dataAttribs.html = cachedImage.html;
+               //console.log('cache hit for ' + token.dataAttribs.src);
+               cb( {tokens: wrapperTokens} );
+               return;
+       }
+
        var env = this.manager.env,
                // distinguish media types
                // if image: parse options
diff --git a/js/lib/ext.core.TemplateHandler.js 
b/js/lib/ext.core.TemplateHandler.js
index b7a5c91..467b3cc 100644
--- a/js/lib/ext.core.TemplateHandler.js
+++ b/js/lib/ext.core.TemplateHandler.js
@@ -55,16 +55,9 @@
                about = this.manager.env.newObjectId();
        // Assign the HTML fragment to the data-parsoid.html on the first
        // wrapper token.
-       // FIXME: figure out why template encapsulation sometimes uses the 
wrapper
-       // data-parsoid, and sometimes that of the first wrapped element!
        toks[0].dataAttribs.html = expansion.html;
        // Add the DOMFragment type so that we get unwrapped later
        toks[0].setAttribute('typeof', 'mw:DOMFragment');
-
-       // FIXME: figure out why template encapsulation sometimes uses the 
wrapper
-       // data-parsoid, and sometimes that of the first wrapped element!
-       toks[0].dataAttribs.html = expansion.html;
-       //console.log(expansion.html, toks);
 
        // Add the about to all wrapper tokens
        toks.forEach(function(tok) {
diff --git a/js/lib/mediawiki.DOMUtils.js b/js/lib/mediawiki.DOMUtils.js
index eed9e9b..d4a91dd 100644
--- a/js/lib/mediawiki.DOMUtils.js
+++ b/js/lib/mediawiki.DOMUtils.js
@@ -39,7 +39,7 @@
                        }
                        node.setAttribute('typeof', types.join(''));
                } else {
-                       node.setAttribute(type);
+                       node.setAttribute('typeof', type);
                }
        },
 
@@ -689,6 +689,12 @@
         *                              html: 'html2',
         *                              nodes: [<node1>, <node2>]
         *                      }
+        *     },
+        *     images: {
+        *         'key3': {
+        *                              html: 'html3',
+        *                              nodes: [<node1>, <node2>]
+        *                      }
         *     }
         * }
         */
@@ -697,7 +703,8 @@
                        expansion,
                        expansions = {
                                transclusions: {},
-                               extensions: {}
+                               extensions: {},
+                               images: {}
                        };
 
                function getAboutSiblings(node, about) {
@@ -722,7 +729,7 @@
                                if (node.nodeType === node.ELEMENT_NODE) {
                                        var typeOf = 
node.getAttribute('typeof'),
                                                about = 
node.getAttribute('about');
-                                       if 
(/\b(?:mw:(?:Transclusion\b|Extension\/))/
+                                       if 
(/\b(?:mw:(?:Transclusion\b|Extension\/|Image(?:\b|\/)))/
                                                        .test(typeOf) && about)
                                        {
                                                DOMUtils.loadDataParsoid(node);
@@ -731,9 +738,15 @@
                                                if 
(/\bmw:Transclusion\b/.test(typeOf)) {
                                                        expAccum = 
expansions.transclusions;
                                                        key = 
node.data.parsoid.src;
-                                               } else {
+                                               } else if 
(/\bmw:Extension\//.test(typeOf)) {
                                                        expAccum = 
expansions.extensions;
                                                        key = 
node.data.parsoid.src;
+                                               } else {
+                                                       expAccum = 
expansions.images;
+                                                       // XXX gwicke: use 
proper key that is not
+                                                       // source-based? This 
also needs to work for
+                                                       // transclusion output.
+                                                       key = 
node.data.parsoid.cacheKey;
                                                }
 
                                                if (key) {
diff --git a/js/lib/mediawiki.Util.js b/js/lib/mediawiki.Util.js
index 9b2f2f4..6d5a46f 100644
--- a/js/lib/mediawiki.Util.js
+++ b/js/lib/mediawiki.Util.js
@@ -1361,6 +1361,7 @@
                if (expansions) {
                        env.transclusionCache = expansions.transclusions;
                        env.extensionCache = expansions.extensions;
+                       env.imageCache = expansions.images;
                }
 
                // Now go ahead with the actual parsing
diff --git a/js/lib/mediawiki.parser.environment.js 
b/js/lib/mediawiki.parser.environment.js
index 9333d8f..ccfbbdf 100644
--- a/js/lib/mediawiki.parser.environment.js
+++ b/js/lib/mediawiki.parser.environment.js
@@ -86,6 +86,9 @@
        // Global extension tag expansion cache (templates, parser functions 
etc)
        // Key: Full extension source (including tags)
        this.extensionCache = {};
+       // Global image expansion cache
+       // Key: Full image source
+       this.imageCache = {};
 
        if ( !parsoidConfig ) {
                // Global things, per-parser

-- 
To view, visit https://gerrit.wikimedia.org/r/66983
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I80139a8e7e4be6d34340f007b2a980b641a4c6eb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Parsoid
Gerrit-Branch: master
Gerrit-Owner: GWicke <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to