From 6f1c46819e38a413eb60cabd528bf11b816cc181 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 9 Oct 2015 01:52:51 +1030 Subject: [PATCH] Minify each JS file individually, caching the result This means that the expensive minification process will only be run for a file if it hasn't before. Greatly speeds up extension enabling/disabling. Also: - Don't check file last modification times in production for a bit of extra perf. - Only flush CSS when theme settings are changed. This speeds up the page reload a bit. --- src/Admin/AdminServiceProvider.php | 13 +++- src/Admin/Controller/ClientController.php | 4 +- src/Asset/AssetManager.php | 6 +- src/Asset/JsCompiler.php | 59 +++++++++++-------- src/Asset/LessCompiler.php | 5 +- src/Asset/RevisionCompiler.php | 40 +++++++++---- src/Forum/Controller/ClientController.php | 16 ++++- src/Forum/ForumServiceProvider.php | 13 +++- .../Controller/AbstractClientController.php | 49 +++++++++++---- src/Locale/JsCompiler.php | 6 +- 10 files changed, 149 insertions(+), 62 deletions(-) diff --git a/src/Admin/AdminServiceProvider.php b/src/Admin/AdminServiceProvider.php index e599d5b1f..114c0d7cc 100644 --- a/src/Admin/AdminServiceProvider.php +++ b/src/Admin/AdminServiceProvider.php @@ -71,7 +71,7 @@ class AdminServiceProvider extends AbstractServiceProvider { $this->app->make('events')->listen(SettingWasSet::class, function (SettingWasSet $event) { if (preg_match('/^theme_|^custom_less$/i', $event->key)) { - $this->flushAssets(); + $this->getClientController()->flushCss(); } }); } @@ -86,7 +86,14 @@ class AdminServiceProvider extends AbstractServiceProvider public function flushAssets() { - $action = $this->app->make('Flarum\Admin\Controller\ClientController'); - $action->flushAssets(); + $this->getClientController()->flushAssets(); + } + + /** + * @return \Flarum\Admin\Controller\ClientController + */ + protected function getClientController() + { + return $this->app->make('Flarum\Admin\Controller\ClientController'); } } diff --git a/src/Admin/Controller/ClientController.php b/src/Admin/Controller/ClientController.php index aa3eb3c89..d5826e181 100644 --- a/src/Admin/Controller/ClientController.php +++ b/src/Admin/Controller/ClientController.php @@ -13,6 +13,7 @@ namespace Flarum\Admin\Controller; use Flarum\Foundation\Application; use Flarum\Http\Controller\AbstractClientController as BaseClientController; use Flarum\Extension\ExtensionManager; +use Illuminate\Contracts\Cache\Repository; use Illuminate\Contracts\Events\Dispatcher; use Psr\Http\Message\ServerRequestInterface as Request; use Flarum\Core\Permission; @@ -47,9 +48,10 @@ class ClientController extends BaseClientController LocaleManager $locales, SettingsRepository $settings, Dispatcher $events, + Repository $cache, ExtensionManager $extensions ) { - BaseClientController::__construct($app, $apiClient, $locales, $settings, $events); + BaseClientController::__construct($app, $apiClient, $locales, $settings, $events, $cache); $this->layout = __DIR__.'/../../../views/admin.blade.php'; $this->extensions = $extensions; diff --git a/src/Asset/AssetManager.php b/src/Asset/AssetManager.php index 4da5fdc46..c4fe144f3 100644 --- a/src/Asset/AssetManager.php +++ b/src/Asset/AssetManager.php @@ -106,9 +106,13 @@ class AssetManager return $this->js->getFile(); } - public function flush() + public function flushCss() { $this->less->flush(); + } + + public function flushJs() + { $this->js->flush(); } } diff --git a/src/Asset/JsCompiler.php b/src/Asset/JsCompiler.php index 97ff18bb1..a07401efa 100644 --- a/src/Asset/JsCompiler.php +++ b/src/Asset/JsCompiler.php @@ -11,54 +11,44 @@ namespace Flarum\Asset; use Exception; +use Illuminate\Cache\Repository; use MatthiasMullie\Minify; use s9e\TextFormatter\Configurator\JavaScript\Minifiers\ClosureCompilerService; class JsCompiler extends RevisionCompiler { /** - * @var bool + * @var Repository */ - protected $minify; + protected $cache; /** * @param string $path * @param string $filename - * @param bool $minify + * @param bool $watch + * @param Repository $cache */ - public function __construct($path, $filename, $minify = false) + public function __construct($path, $filename, $watch = false, Repository $cache = null) { - parent::__construct($path, $filename); + parent::__construct($path, $filename, $watch); - $this->minify = $minify; + $this->cache = $cache; } /** * {@inheritdoc} */ - public function format($string) + protected function format($string) { - return $string.";\n"; - } + if (! $this->watch) { + $key = 'js.'.sha1($string); - /** - * @inheritDoc - */ - public function compile() - { - $output = parent::compile(); - - if ($this->minify) { - set_time_limit(60); - - try { - $output = $this->minifyWithClosureCompilerService($output); - } catch (Exception $e) { - $output = $this->minifyWithFallback($output); - } + $string = $this->cache->rememberForever($key, function () use ($string) { + return $this->minify($string); + }); } - return $output; + return $string.";\n"; } /** @@ -66,7 +56,24 @@ class JsCompiler extends RevisionCompiler */ protected function getCacheDifferentiator() { - return $this->minify; + return $this->watch; + } + + /** + * @param string $source + * @return string + */ + protected function minify($source) + { + set_time_limit(60); + + try { + $source = $this->minifyWithClosureCompilerService($source); + } catch (Exception $e) { + $source = $this->minifyWithFallback($source); + } + + return $source; } /** diff --git a/src/Asset/LessCompiler.php b/src/Asset/LessCompiler.php index b846cb3d8..1f4466042 100644 --- a/src/Asset/LessCompiler.php +++ b/src/Asset/LessCompiler.php @@ -23,11 +23,12 @@ class LessCompiler extends RevisionCompiler /** * @param string $path * @param string $filename + * @param bool $watch * @param string $cachePath */ - public function __construct($path, $filename, $cachePath) + public function __construct($path, $filename, $watch, $cachePath) { - parent::__construct($path, $filename); + parent::__construct($path, $filename, $watch); $this->cachePath = $cachePath; } diff --git a/src/Asset/RevisionCompiler.php b/src/Asset/RevisionCompiler.php index c3e9d78f6..80f816779 100644 --- a/src/Asset/RevisionCompiler.php +++ b/src/Asset/RevisionCompiler.php @@ -24,14 +24,21 @@ class RevisionCompiler implements CompilerInterface */ protected $strings = []; + /** + * @var bool + */ + protected $watch; + /** * @param string $path * @param string $filename + * @param bool $watch */ - public function __construct($path, $filename) + public function __construct($path, $filename, $watch = false) { $this->path = $path; $this->filename = $filename; + $this->watch = $watch; } /** @@ -63,18 +70,21 @@ class RevisionCompiler implements CompilerInterface */ public function getFile() { - $old = $this->getRevision(); - - $lastModTime = 0; - foreach ($this->files as $file) { - $lastModTime = max($lastModTime, filemtime($file)); - } - - $current = hash('crc32b', serialize([$lastModTime, $this->getCacheDifferentiator()])); + $old = $current = $this->getRevision(); $ext = pathinfo($this->filename, PATHINFO_EXTENSION); $file = $this->path.'/'.substr_replace($this->filename, '-'.$old, -strlen($ext) - 1, 0); + if ($this->watch) { + $cacheDifferentiator = [$this->getCacheDifferentiator()]; + + foreach ($this->files as $source) { + $cacheDifferentiator[] = [$source, filemtime($source)]; + } + + $current = hash('crc32b', serialize($cacheDifferentiator)); + } + $exists = file_exists($file); if (! $exists || $old !== $current) { @@ -83,6 +93,7 @@ class RevisionCompiler implements CompilerInterface } $this->putRevision($current); + $file = $this->path.'/'.substr_replace($this->filename, '-'.$current, -strlen($ext) - 1, 0); file_put_contents($file, $this->compile()); } @@ -115,7 +126,7 @@ class RevisionCompiler implements CompilerInterface $output = ''; foreach ($this->files as $file) { - $output .= $this->format(file_get_contents($file)); + $output .= $this->formatFile($file); } foreach ($this->strings as $callback) { @@ -125,6 +136,15 @@ class RevisionCompiler implements CompilerInterface return $output; } + /** + * @param string $file + * @return string + */ + protected function formatFile($file) + { + return $this->format(file_get_contents($file)); + } + /** * @return string */ diff --git a/src/Forum/Controller/ClientController.php b/src/Forum/Controller/ClientController.php index ef3bf8fef..5c05ded46 100644 --- a/src/Forum/Controller/ClientController.php +++ b/src/Forum/Controller/ClientController.php @@ -11,10 +11,12 @@ namespace Flarum\Forum\Controller; use Flarum\Api\Client; +use Flarum\Formatter\Formatter; use Flarum\Foundation\Application; use Flarum\Settings\SettingsRepository; use Flarum\Locale\LocaleManager; use Flarum\Http\Controller\AbstractClientController; +use Illuminate\Contracts\Cache\Repository; use Illuminate\Contracts\Events\Dispatcher; class ClientController extends AbstractClientController @@ -29,6 +31,11 @@ class ClientController extends AbstractClientController */ protected $translationKeys = ['core.forum', 'core.lib']; + /** + * @var Formatter + */ + protected $formatter; + /** * {@inheritdoc} */ @@ -37,11 +44,14 @@ class ClientController extends AbstractClientController Client $api, LocaleManager $locales, SettingsRepository $settings, - Dispatcher $events + Dispatcher $events, + Repository $cache, + Formatter $formatter ) { - parent::__construct($app, $api, $locales, $settings, $events); + parent::__construct($app, $api, $locales, $settings, $events, $cache); $this->layout = __DIR__.'/../../../views/forum.blade.php'; + $this->formatter = $formatter; } /** @@ -52,7 +62,7 @@ class ClientController extends AbstractClientController $assets = parent::getAssets(); $assets->addJs(function () { - return $this->app->make('flarum.formatter')->getJs(); + return $this->formatter->getJs(); }); return $assets; diff --git a/src/Forum/ForumServiceProvider.php b/src/Forum/ForumServiceProvider.php index 9bc3c5b47..ad6d84cf2 100644 --- a/src/Forum/ForumServiceProvider.php +++ b/src/Forum/ForumServiceProvider.php @@ -148,7 +148,7 @@ class ForumServiceProvider extends AbstractServiceProvider { $this->app->make('events')->listen(SettingWasSet::class, function (SettingWasSet $event) { if (preg_match('/^theme_|^custom_less$/i', $event->key)) { - $this->flushAssets(); + $this->getClientController()->flushCss(); } }); } @@ -163,7 +163,14 @@ class ForumServiceProvider extends AbstractServiceProvider public function flushAssets() { - $controller = $this->app->make('Flarum\Forum\Controller\ClientController'); - $controller->flushAssets(); + $this->getClientController()->flushAssets(); + } + + /** + * @return \Flarum\Forum\Controller\ClientController + */ + protected function getClientController() + { + return $this->app->make('Flarum\Forum\Controller\ClientController'); } } diff --git a/src/Http/Controller/AbstractClientController.php b/src/Http/Controller/AbstractClientController.php index c7a425a9b..5d6ee9de6 100644 --- a/src/Http/Controller/AbstractClientController.php +++ b/src/Http/Controller/AbstractClientController.php @@ -21,6 +21,7 @@ use Flarum\Foundation\Application; use Flarum\Locale\JsCompiler as LocaleJsCompiler; use Flarum\Locale\LocaleManager; use Flarum\Settings\SettingsRepository; +use Illuminate\Contracts\Cache\Repository; use Illuminate\Contracts\Events\Dispatcher; use Psr\Http\Message\ServerRequestInterface as Request; @@ -90,25 +91,33 @@ abstract class AbstractClientController extends AbstractHtmlController */ protected $events; + /** + * @var Repository + */ + protected $cache; + /** * @param \Flarum\Foundation\Application $app * @param Client $api * @param LocaleManager $locales * @param \Flarum\Settings\SettingsRepository $settings * @param Dispatcher $events + * @param Repository $cache */ public function __construct( Application $app, Client $api, LocaleManager $locales, SettingsRepository $settings, - Dispatcher $events + Dispatcher $events, + Repository $cache ) { $this->app = $app; $this->api = $api; $this->locales = $locales; $this->settings = $settings; $this->events = $events; + $this->cache = $cache; } /** @@ -161,13 +170,27 @@ abstract class AbstractClientController extends AbstractHtmlController /** * Flush the client's assets so that they will be regenerated from scratch * on the next render. - * - * @return void */ public function flushAssets() { - $this->getAssets()->flush(); + $this->flushCss(); + $this->flushJs(); + } + public function flushCss() + { + $this->getAssets()->flushCss(); + } + + public function flushJs() + { + $this->getAssets()->flushJs(); + + $this->flushLocales(); + } + + public function flushLocales() + { $locales = array_keys($this->locales->getLocales()); foreach ($locales as $locale) { @@ -180,15 +203,16 @@ abstract class AbstractClientController extends AbstractHtmlController * compiler. Automatically add the files necessary to boot a Flarum client, * as well as any configured LESS customizations. * - * @return \Flarum\Asset\AssetManager + * @return AssetManager */ protected function getAssets() { $public = $this->getAssetDirectory(); + $watch = $this->app->config('debug'); $assets = new AssetManager( - new JsCompiler($public, "$this->clientName.js", ! $this->app->config('debug')), - new LessCompiler($public, "$this->clientName.css", $this->app->storagePath().'/less') + new JsCompiler($public, "$this->clientName.js", $watch, $this->cache), + new LessCompiler($public, "$this->clientName.css", $watch, $this->app->storagePath().'/less') ); $this->addAssets($assets); @@ -201,7 +225,7 @@ abstract class AbstractClientController extends AbstractHtmlController * Add the assets necessary to boot a Flarum client, found within the * directory specified by the $clientName property. * - * @param \Flarum\Asset\AssetManager $assets + * @param AssetManager $assets */ protected function addAssets(AssetManager $assets) { @@ -214,7 +238,7 @@ abstract class AbstractClientController extends AbstractHtmlController /** * Add any configured JS/LESS customizations to the asset manager. * - * @param \Flarum\Asset\AssetManager $assets + * @param AssetManager $assets */ protected function addCustomizations(AssetManager $assets) { @@ -255,7 +279,12 @@ abstract class AbstractClientController extends AbstractHtmlController */ protected function getLocaleCompiler($locale) { - $compiler = new LocaleJsCompiler($this->getAssetDirectory(), "$this->clientName-$locale.js"); + $compiler = new LocaleJsCompiler( + $this->getAssetDirectory(), + "$this->clientName-$locale.js", + $this->app->config('debug'), + $this->cache + ); foreach ($this->locales->getJsFiles($locale) as $file) { $compiler->addFile($file); diff --git a/src/Locale/JsCompiler.php b/src/Locale/JsCompiler.php index 78a15bef3..6cac02c79 100644 --- a/src/Locale/JsCompiler.php +++ b/src/Locale/JsCompiler.php @@ -10,9 +10,9 @@ namespace Flarum\Locale; -use Flarum\Asset\RevisionCompiler; +use Flarum\Asset\JsCompiler as BaseJsCompiler; -class JsCompiler extends RevisionCompiler +class JsCompiler extends BaseJsCompiler { protected $translations = []; @@ -36,6 +36,6 @@ class JsCompiler extends RevisionCompiler }; });"; - return $output; + return $this->format($output); } }