The patch did not improve performance. I'd like to think that the increased
control over when inline scripts are executed makes the patch worthwhile
regardless, but that is post hoc justification and possibly a bit of personal
ego. Krinkle agrees that we may use some of the ideas in this patch in the
future but he thinks we're better off not heading down this path before we
have a better sense of where we're going, and I trust his judgment.
This reverts commit e86e5f8460.
Change-Id: I151f74a41dd664b5a0aa5cfd99fcc95e2686a1e6
The current ordering of scripts and stylesheets in <head> causes all major
browsers to serialize and defer requests that could be performed in parallel.
The problem is that external stylesheets are loaded before inline scripts. As
Steven Souders explains, "all major browsers preserve the order of CSS and
JavaScript. The stylesheet has to be fully downloaded, parsed, and applied
before the inline script is executed. And the inline script must be executed
before the remaining resources can be downloaded. Therefore, resources that
follow a stylesheet and inline script are blocked from downloading."[1]
In other words: the browser could start loading body images, but it refuses to
do that until it has executed inline scripts in head. And it refuses to execute
those scripts until the external CSS is downloaded, parsed and applied. You can
see the effect of this in this image, showing the request waterfall for
[[en:Gothic Alphabet]]: [2]. Notice how no images were requested before the
browser had finished processing the three load.php requests at the top.
To fix this, we want to move the inline scripts above the external CSS. This is
a little bit tricky, because the inline scripts depend on mw.loader, which is
loaded via an external script. If we move the external script so that it too is
above the external stylesheet, we force the browser to serialize requests,
because the browser will not retrieve the external CSS until it has retrieved
and executed the external JS code. So what we want is to move the inline
scripts above the external stylesheet, but keep the external script (which the
inline scripts depend on) below the external stylesheet.
We can do this by wrapping the inline script code in a closure (which binds
'mw') and enqueuing the closure in a global array which will be processed by
the startup module at just the right time.
Net result: external CSS and JS is retrieved in parallel, retrieval of images
(and other external assets) is unblocked, but the order in which code is
evaluated remains the same.
[1]: <http://www.stevesouders.com/blog/2009/05/06/positioning-inline-scripts/>
[2]: <http://people.wikimedia.org/~ori/enwiki-waterfall.png> (excerpted from
<http://www.webpagetest.org/result/150316_0C_7MB/1/details/>.
Change-Id: I98d383a6299ffbd10210431544a505338ca8643f
Xhprof generates this data now. Custom profiling of various
sub-function units are kept.
Calls to profiler represented about 3% of page execution
time on Special:BlankPage (1.5% in/out); after this change
it's down to about 0.98% of page execution time.
Change-Id: Id9a1dc9d8f80bbd52e42226b724a1e1213d07af7
We already did this, but it was rather convoluted with lots of
if/elseif sequences checking all the possible values.
Remove this logic from ResourceLoaderStartUpModule. Simplying it
simply create the array and pass it to ResourceLoader::makeLoaderRegisterScript.
In makeLoaderRegisterScript, we apply a filter to the array(s) that
trim empty values.
While at it:
* As with other registration properties' default values (like for dependencies,
group, and skip) also use 'null' for the default value of 'source'.
The mediawiki.js client was already compatible with this, and the server omitted
it if it was the last value in the list. But in all other cases it explicitly
outputs "local". Use null instead of simplicity sake. This also gains us a
few characters in the output, and a relatively larger win after gzip since
there's lots more re-using of "null".
* Remove stray casting of $version to int. This only happened in case of
registering a single module (which don't do anywhere), and is redundant.
Change-Id: I1f321e7b8bd3b5cffc550b51169957a3da9b971d
By using the existing indexes of modules in the array being passed to
mw.loader.register we can reduce the size of the startup module by about
6% after gzip (nearly 20% before) on a wiki with very few modules (such
as my localhost). Comparing data from en.wikipedia.org shows about 9%
after gzip (nearly 30% before).
The technique adds a function to mediawiki.js which resolves the indexes
before registering the modules, which costs a little bit of data in that
payload, but it's negligible (118 bytes after gzip) in comparison to the
overall reduction.
Also, cleaned up lies in documentation and strange use of "m" as an
iterator variable.
Bonus: fix ISO8601 timestamp instead of UNIX timestamp being passed
to custom loader scripts.
Change-Id: If12991413fa6129cd20ceab0e59a3a30a4fdf5ce
* Use time() instead of:
- wfTimestamp()
- wfTimestamp( TS_UNIX )
- wfTimestamp( TS_UNIX, 0 )
- wfTimestamp( TS_UNIX, time() )
- intval( wfTimestamp( TS_UNIX, time() ) )
* Consistently use 1 as default instead of 0. Previously the
unwritten convention was that anything "final" max()'ed with 1,
and any internal method would use 0, but this wasn't applied
consistently made it fragile. There doesn't seem to be any
value in returning 0 only to have it maxed up to 1 (because if
the 0 would ever make it out alive, we'd be in trouble).
* wfTimestamp returns a string for TS_UNIX. In PHP this doesn't
matter much. In fact, max() takes number-like integers so
transparently, it even preserves it:
> max( 1, 3, '2' );
< 3
> max( 1, '3', 2 );
< "3"
Just cast it in one place at the very end (StartupModule)
instead of doing intval( wfTimestamp() ).
* Fix weird documentation claiming getModifiedTime can return
an array, or mixed.
* Remove 'version > 1 ? version : 1' logic in
ResourceLoader::makeLoaderRegisterScript. The client doesn't
have "0 means now" behaviour so this isn't needed. And the method
was only doing it for variadic argument calls.
Removal of quotes around timestamps reduced the size of the startup
module from 26.8KB to 25.9KB before gzip. After gzip the size was
and still is 5.7KB, though. (From 5456 bytes to 5415 bytes.)
Change-Id: If92ca3e7511e78fa779f2f2701e2ab24db78c8a8
site-wide
UploadWizard also uses wgFileCanRotate and set the variable by
I3c50b56e6a11742cfc1c02d3d8dbeb0c151b0e91 itself.
Change-Id: Iff6a952c8f4f21c0e27bd6b4191f3b0b453eec03
BitmapHandler has a lot of generic-ish functionality that could
be re-usable by extension classes (Such as how it organizes
$scalerParams array, or various image magick escaping methods).
However it's combined with a lot of very format specific things,
such as the shell-out call to image magick.
Try to separate out the more generic stuff into
TransformationalImageHandler. In order to do this, I also made
canRotate, autoRotateEnabled, and getScalerType non-static. No
extensions in our repo appeared to be using these methods, and they
don't really make sense to be static (imo).
In particular, I think code duplication can be reduced in
PagedTiffHandler by extending this new class. See comments
on I1b9a77a4a56eeb65.
Change-Id: Id3a8b25a598942572cb5791a95e86054d7784961
Prevents modules from depending on raw modules, like 'mediawiki' & 'jquery'.
To do that, those modules (as designated by raw=>true), aren't
registered in the module registry.
Change-Id: Iac37a236964dc40d6259988a0baaaa6331b54ec6
We currently have a few legacy requests to the load.php end point
that bypass the ResourceLoader client by coding a request to
load.php via a "<script src>" directly. Most prominently the
request for the 'site' wiki module (aka MediaWiki:Common.js).
Remove the manual wrapping of embedded private modules as this
is now taken are of by ResourceLoader::makeModuleResponse itself.
Misc:
* Mark "jquery" and "mediawiki" as Raw modules. While the startup
module had this already, these didn't. Without this, they'd
get the conditional wrap – which would be a problem since mediawiki.js
can't be conditional on 'window.mw' for that file defines that
namespace itself.
* Strip the cache-key comment in the unit tests because the hash
no longer matches and using the generic 'wiki' dbname was breaking
DB queries.
* Relates to bug 63587.
* See also 05d0f6fefd which expands the reach of the non-JS
environment to IE6 and helped expose this bug.
Change-Id: Icf6ede09b51ce212aa70ff6be4b341762ec75b4d
- Swap "$variable type" to "type $variable"
- Added missing types
- Fixed spacing inside docs
- Makes beginning of @param/@return/@var/@throws in capital
- Changed some types to match the more common spelling
Change-Id: I8ebfbcea0e2ae2670553822acedde49c1aa7e98d
- Removed spaces after not operator (!)
- Removed spaces inside array index
- use tab as indent instead of spaces
- Add newline at end of file
- Removed spaces after casts
Change-Id: I9ba17c4385fcb43d38998d45f89cf42952bc791b
Deprecated only since 1.23, but universally reviled, and used only by
MwEmbedSupport, which no longer uses it as of I43af3c87a).
Change-Id: I36c89b273fdb0ec3a76034c7a6d2f48a15d5457f
A module can be registered with a skip function. Such function,
if provided, will be invoked by the client when a module is
queued for loading. If the function returns true, the client will
bypass any further loading action and mark the module as 'ready'.
This can be used to implement a feature test for a module
providing a shim or polyfill.
* Change visibility of method ResourceLoader::filter to public.
So that it can be invoked by ResourceLoaderStartupModule.
* Add option to suppress the cache key report in ResourceLoader::filter.
We usually only call the minifier once on an entire request
reponse (because it's all concatenated javascript or embedded
javascript in various different closures, still valid as one
large script) and only add a little bottom line for the cache
key. When embedding the skip function we have to run the minifier
on them separately as they're output as strings (not actual
functions). These strings are typically quite small and blowing
up the response with loads of cache keys is not desirable in
production.
* Add method to clear the static cache of ResourceLoader::inDebugMode.
Global static state is evil but, as long as we have it, we at
least need to clear it after switching contexts in the test suite.
Also:
* Remove obsolete setting of 'debug=true' in the FauxRequest in
ResourceLoaderTestCase. It already sets global wgResourceLoaderDebug
in the setUp() method.
Bug: 66390
Change-Id: I87a0ea888d791ad39f114380c42e2daeca470961
Provides functonality similar to WebRequest#getCookie and WebResponse#setcookie.
Wraps $.cookie and automatically takes care of wgCookiePrefix etc.
Bug: 49156
Change-Id: I217ef258aecf1acd335e2cea56ae08b22541c7d4
Co-Author: Matthew Flaschen <mflaschen@wikimedia.org>
Co-Author: Timo Tijhof <krinklemail@gmail.com>
This partially reverts r73950 which removed $wgServerName on the ground that it
was only used for {{SERVERNAME}}. When it was pointed out that $wgServerName was
also used by several extensions, the response was not to restore the variable, but
to proceed to remove it from extensions as well.
It is a useful variable to have, as the discussion on Id819246a9 makes clear
(see Tim's comment on PS12 and Timo's reply). So let's reintroduce it, and expose
it in mw.config and ApiQuerySiteInfo as well.
Change-Id: I40a6fd427d38c64c628f70a2f407b145443ea204
The optimization basically works like this:
* Given module A with the dependencies B and C and module B with the
dependency C.
* Don't tell the client that A depends on C, as that's already included
in module B.
This way we can reduce the amount of data for module registration sent
to the client.
The code here isn't polished yet, but it works and should be good enough
to demonstrate my idea and implementation.
Change-Id: I7732a3b1d879c5eef059e136a5241d6d48046872
Swapped some "$var type" to "type $var" or added missing types
before the $var. Changed some other types to match the more common
spelling. Makes beginning of some text in captial.
Change-Id: Ifbb1da2a6278b0bde2a6f6ce2e7bd383ee3fb28a
In bug 63240, Roan calls it a 'misfeature', which I think is exactly right. It
has been used as a lazy workaround for having to think about load-order,
dependency management, and race conditions.
Bug: 63240
Change-Id: Ic48ad39c6d3d166d6bb8f60dfdd5f95aa7ed16a6
This will make it easier to debug this code and generally to interact
with the registry data before it gets sent out.
Change-Id: I01808ce5d6a0ac6f7a15719bdd2aa90908cb2fbd
- Added spaces after if/foreach/catch
- Added new line before end of file
- Added or removed spaces before/after parenthesis, comma
- Added spaces around string concat
Change-Id: I0590070f1b3542108e242730e8d9a3ba9831e94f
Changes to the static startup.js file are covered by the file
mtime in #getModifiedTime.
Changes to the module registrations are covered by the max
module mtime loop in #getModifiedTime.
But changes to LocalSettings.php etc. affecting mw.config values
embedded in #getScript aren't traceable. Resort to hashing those
and detecting changes between calls that way.
Without this, changes to mw.config never made it live (not even
after the rebuild every 5 minutes) because of 304 Not Modified
being sent between Apache and Varnish. Changes to config either
didn't matter to front-end right away and made it with the next
deployment that changes a different module, or by touching
startup.js to force a version bump.
Simple third party installs were not affected by this bug as their
settings would only be coming from LocalSettings.php (as opposed
to included files) and $wgInvalidateCacheOnLocalSettingsChange is
true by default.
Bug: 28899
Change-Id: I484166923d97368d95beeb40bb209dec9b849033
* Re-ordering code to bring the modifiedTime hack and the
loop it serves closer together.
* Separating the fact that it needs a value and the mtime of
startup.js since that is just one of the three factors we use.
This way it's clearer that the startup.js mtime is not just a
bogus value, but not more or less important than wgCacheEpoch
and modules mtime either.
* Remove duplicate '/* Methods */' comment, we already have this
marker a few methods higher up.
Change-Id: Id3a07f02566c0f04b612b81f8353f70fa4ab3977
Due to the way this function being defined as a function or var
declaration instead of a function expression assigned to a property
it can't be deleted.
JavaScript doesn't throw an error when deletion is not permitted
though, the operator returns false instead.
> delete isCompatible;
false
We already removed 'delete startUp' from mediawiki.js in favour
of startUp = undefined; (r107402, r74325).
Change-Id: I7aa02e3f4deb3a4f00177b70978bfcb83c80988a
Allows for the search interface to be customzied for the particular
search-engine used, and allows collecting client-side performance measurements
that specify which search engine was used.
Change-Id: Ibeda834e9d5dbaf1d7e40c2dacbc60feb2cc4bba
Split the variable assignment and the return statement in two lines for
better readability.
When there was two return statements in one method the logic was swapped
to have only one return statement.
Change-Id: Id7a01b4a2df96036435f9e1a9be5678dd124b0af
Intended to be used in WikiEditor to hide the signature button in content
namespaces. Other uses could be for gadgets or extensions that only need
to work on content pages, or that are internally-facing and should only
be seen on non-content pages.
It is an array of namespace IDs, so that end-users can quickly check
whether wgNamespaceNumber is in this array.
Change-Id: I01d9671dd14aa79e79ed887299c2f5de766f7375
To minimize the number of discrete requests that the browser has to make in
order to render the page, ResourceLoader tries to condense as many modules as
possible into each request. To ensure that the response is not stale,
ResourceLoader tacks the modification time of the most recently modified module
to the request. This behavior makes poor use of locality: an update to a single
module will change the URL that is used to retrieve it and a number of
unrelated modules, causing those modules to be re-retrieved even though they
have not changed since they were last retrieved. This is because the browser
cache is not aware that the response from load.php is a composite of modules
that should be versioned separately.
This patch adds mw.loader.store. On browsers that implement the localStorage
API, the module store serves as a smart complement to the browser cache. Unlike
the browser cache, the module store can slice a concatenated response from
ResourceLoader into its constituent modules and cache each of them separately,
using each module's versioning scheme to determine when the cache should be
invalidated.
Because the localStorage API is synchronous and slower than memory access,
modules are cached as a single JSON blob. At the beginning of the page load
process, the entire blob is loaded from storage into memory. During the load
process, any required modules that are not in the store are fetched from the
server and set in the in-memory store. When the DOM is complete, the store is
synced back to localStorage in a single operation.
* NOTE: The module store is enabled only if $wgResourceLoaderStorageEnabled is
set to true; it is false by default. We can change the default if / when we
establish conclusively that the feature is beneficial and stable.
Change-Id: If2ad2d80db2336fb447817f5c56599667141ec66
If a file type was added to $wgFileExtensions by both local configuration
and defaults in an extension (eg TimedMediaHandler and LocalSettings.php
both adding 'ogg' and 'ogv') it was being listed twice in the UI messages
listing acceptable types.
Runs array_unique() over the array on various outputs.
Bug: 54378
Change-Id: I14cd098d8b27099f8f803630535f33549740295c
The upcoming rewrite of mw.Title needs to use wgLegalTitleChars,
but for that to work, it needs to be converted into something
that can work in javascript.
Signed-off-by: Timo Tijhof <krinklemail@gmail.com>
Signed-off-by: David Chan <david@sheetmusic.org.uk>
Change-Id: I163f3d7e3a680d52640a93f4bd195d8209669918