time latency per file - Githubissues

andkaran commented 10 years ago

Average time taken to load per file is around 1 second for me! does anybody else facing this even on fresh install

evantishuk commented 10 years ago

I would guess this is because it's recompiling your less/css/scss/javascript/coffeescript/ each page load. If you're not actively working with your SCSS/SASS + JS you can turn caching on: https://github.com/CodeSleeve/asset-pipeline/blob/master/src/config/config.php#L110

andkaran commented 10 years ago

But the same thing also happens if I load precompiled coffee, less, handlebars files etc. Cache feature solves the problem only when used with concatenation.

Cache used alone gives more or less the same latency. I am learning and building a Single page app with lots of scattered js files in different modules. I can't turn on cache yet.

Can there be any method to first check if original files have changed before compiling and then send back a 304 not modified header if nothing changed in a particular file?

This could reduce the load times when in active development. On Sep 15, 2013 11:10 AM, "evantishuk" notifications@github.com wrote:

I would guess this is because it's recompiling your less/css/scss/javascript/coffeescript/ each page load. If you're not actively working with your SCSS/SASS + JS you can turn caching on: https://github.com/CodeSleeve/asset-pipeline/blob/master/src/config/config.php#L110

— Reply to this email directly or view it on GitHubhttps://github.com/CodeSleeve/asset-pipeline/issues/38#issuecomment-24464857 .

evantishuk commented 10 years ago

"Can there be any method to first check if original files have changed before compiling and then send back a 304 not modified header if nothing changed in a particular file?"

I would hope that it worked like that out-of-the-box--given that it's using Assetic which provides a simple built-in caching mechanism (see: https://github.com/kriswallsmith/assetic/blob/master/README.md#internal-caching).

Looking through the code, it seems Asset Pipeline has some concept of caching as evidenced here:

https://github.com/CodeSleeve/asset-pipeline/blob/63c57f3edf0581d8ff0b915321aa0eca1e7a1187/src/Codesleeve/AssetPipeline/AssetCacheRepository.php#L54

But that doesn't quite cut the mustard and doesn't appear to use Assetic's built in AssetCache.

So, I played around and took a shot at weaving in Assetics Caching approach , at around this part of the SprocketsRepository:

Using the above lines as a reference point, you can edit the relevant logic to look something like:

foreach ($files as $file)
{   
    $base = $this->basePath($file);
    if ($this->isJavascript($base))
    {
        $filters = $this->filters->matching($file);
        $assets[] = new AssetCache(
            new FileAsset($this->getFullPath($base, 'javascripts'), $filters),
            new FileSystemCache(\Config::get('cache.path'));
    }
}

NOTES:

You need to add use Assetic\AssetCache; and use Assetic\FilesystemCache; at the beginning of the SprocketsRepository.php file as well.
The above snippet edits the getScriptAssets function. To complete the job, you also need to change the corresponding getStyleAssets function as well.
Because I did this fast, you may notice that it's hitting your Config file for every request ... a more robust solution would need to avoid that.

So, I thought this would work as-is. But, alas, it exposed what looks like a legitimate issue with Assetic whereby the MinifyCSS filter causes an exception to be thrown due to its having a closure. I looked into that and isolated the cause and started a discussion around a patch. You may wish to join that conversation here: https://github.com/kriswallsmith/assetic/issues/501

So, if you don't want to wait around for the discussion, pull requests, and possible bug fixes before an eventual release (or not)... you can make the changes I suggest above AND the changes I propose over on the Assetic issues list (https://github.com/kriswallsmith/assetic/issues/501), and it should work. I haven't fully tested it, but, it seems to not break and improves loading times.

I am also not the maintainer here and I may be missing something big and obvious and stupid. So, please don't take my suggestions as anything more than the product of 45 minutes worth of poking around from a guy in the peanut gallery.

Cheers.

EDIT: Updated the notes.

kdocki commented 10 years ago

@evantishuk I actually didn't know about the internal caching from Assetic. I can definitely see the usefulness in caching on per file basis if you had lots of files that needed pre-processing. Also, I don't know how this would mesh with this proposal

evantishuk commented 10 years ago

@kdocki from what I can tell, I don't think it would impact that proposal too much, if at all. Assetic's AssetCache looks at the pre-compiled file-data and determines if it needs to recompile or pass along the cached content. So, at worst you may have duplicate file-data between your cache folder and public/assets/ which isn't too big of a deal.

That seems like a good default behavior regardless of how many assets you're wrangling. Especially when using something like Bootstrap Sass which has a lot of overhead with the current approach.

Of course, Assetic's AssetCache doesn't handle the MinifyCSS filter at present, so anything in that direction would have to wait for a fix on their end or come up with a workaround that avoids the whole closure issue.

andkaran commented 10 years ago

Jut tried it. There seems to be some improvement, but I feels it still not enough. From old avg load times of 24s has come down to 19s. Well its still huge for a local dev SPA . Lets hope something better comes out of this

felixkiss commented 10 years ago

@karanits Which filters do you have configured and which are actually used on these time consuming requests?

andkaran commented 10 years ago

I have configured to use only Coffeescript and handlebars only. I have removed all non necessary paths also

felixkiss commented 10 years ago

What Handlebar filter class do you use?

Can you narrow it down to one of them? Maybe takeout the handlebars filter and try again?

andkaran commented 10 years ago

actually the filters doesnt matter. I had once pre-complied all coffeescripts and handlebar templates before hand and just used this to load the resulting js files. results being the same latency.

i will try to do that again and see what happens

andkaran commented 10 years ago

FYI I am using codesleeve's handlebar plug-in. Before that I used to one provided by Assetic

andkaran commented 10 years ago

I just tried only loading pre compiled coffee files. Used no filters. Same type of latency.

I also hard wrote the generated script tags in my blade view and copied all compiled coffee files to public folder. Instantaneous page reload... Obviously! I know this was not required but still did it just so.

This just leaves one thing. File system is slowing things down.

evantishuk commented 10 years ago

Sorry, I'm a little confused now. If you employed the AssetCache as above, it shouldn't recompile files if nothing changed. But, as you've explained, even when its precompiled and devoid of filters it's still slow. It's starting to sound like a peculiarity of your environment. Are you remote mounting to a development server or something?

Can you publish an archive of your assets and structure so we can try to reproduce?

andkaran commented 10 years ago

Yup. That's what I am thinking. I have a laptop running windows 8 and Ubuntu server running in it virtually. I do coding in win 8 and push to the virtual server. So the website is technically severed on a different IP address

A few days ago, I had wampserver on windows 8, before going virtual. Then this was all on localhost. Same issue was there to. May be its my laptop issue?

Well my asset structure is similar to backbonerails.Com tutorials by Brain Mann. And he makes a lot of folders!

I am loading atleast 20 js files and most of them are less than 100 lines. Can anyone test just by loading some random js files?

I am sorry, I feel I am not helpful enough.

evantishuk commented 10 years ago

When I get a chance, I'll try including a few dozen dummy plain JS and coffee files. My local environment is just straight-up Ubuntu, so it should provide a relatively "clean" test. I can also try on my remote dev server which has a lot more horsepower and see how that responds. Even if it turns out that your environment is likely to blame, this discussion (I think) could lead to some real improvements!

evantishuk commented 10 years ago

@kdocki a fellow over on the Assetic project pointed out that the HashableInterface should be used with any (all?) filters--which may impact how you want to structure the filters in order to reliably leverage Assetic's built in caching and avoid the closure serialization issue. Here's the basic solution using MinifyCSS as an example:

First, adjust the SprocketsRepository to use Assetic's AssetCache (https://github.com/CodeSleeve/asset-pipeline/issues/38#issuecomment-24479237), from above:

foreach ($files as $file)
{   
    $base = $this->basePath($file);
    if ($this->isJavascript($base))
    {
        $filters = $this->filters->matching($file);
        $assets[] = new AssetCache(
            new FileAsset($this->getFullPath($base, 'javascripts'), $filters),
            new FileSystemCache(\Config::get('cache.path'));
    }
}

Second, the filters should implement HashableInterface. The only way I can think to do this is to extend the current class and implement HashableInterface from there. So, in our case, MinifyCSS becomes MinifyCSSBase and stays unchanged otherwise. Then we create a new MinifyCSS class that looks like:

<?php namespace Codesleeve\AssetPipeline\Filters;

# /asset-pipeline/src/CodeSleeve/AssetPipeline/Filters/MinifyCSS.php

use Assetic\Filter\HashableInterface;

class MinifyCSS extends MinifyCSSBase implements HashableInterface {

  public function hash() { return "MinifyCSS"; }

}

Maybe there's a better way to structure it. I've tested this approach and it works like a charm. If you have a better way to organize and implement the HashableInterface, let me know. Otherwise, I'd be happy to adapt these filters and make a pull request.

kdocki commented 10 years ago

@evantishuk Thanks for looking into this! :) Did you see an issue too? Also did this HashableInterface will fix the issue if you were able to reproduce it? If so and you want to make those changes ... put in a pull request and I'll definitely push it through.

kdocki commented 10 years ago

Well... I am going to try to put in some of these changes @evantishuk mentioned and test it... I'm not familar with Assetic Cache stuff but I'd like to see if it helps any.

@karanits right now one thing you can do is set concat => true in the config which I think will help your performance. I have ~200 asset files (I too follow Brian Mann's patterns) and I'm seeing load times of about 9 seconds with 'concat' => null and about 1 second loads for 'concat' => true

andkaran commented 10 years ago

Concatenation does bring down the load time. But it makes debugging a lot harder! It brought it down to 10 sec.

Out of frustration I exported the virtual server to another pc and there load times was 6sec. With concatenation

So some of the issue may be with my laptop being over worked. But still I expect that load times should not be more than a few seconds even when not concatenated.

evantishuk commented 10 years ago

Did you see an issue too?

No, I haven't been able to reproduce the issue as described. :( I can rope in a bunch of JS and CSS and it will indeed chunk about 2 - 4 seconds (annoying, but acceptable IMHO). Nothing in the 20+ second range though.

Also did this HashableInterface will fix the issue if you were able to reproduce it?

I don't know. It definitely improves performance in the average case though.

If so and you want to make those changes ... put in a pull request and I'll definitely push it through.

Karantis stated earlier that the previously suggested monkey-patch I suggested offered a small improvement but not a complete fix. General mood was that something peculiar to his I/O was responsible for the delay. I'm leaning toward his case being rare unless another issue like this gets reported.

I am going to try to put in some of these changes @evantishuk mentioned and test it... I'm not familiar with Assetic Cache stuff but I'd like to see if it helps any.

Let me know how that goes. I think it's the "right" way to do it. I suppose it should probably be the default behavior with a boolean config variable to turn it off in debug mode or explicitly. Also, I'm curious how best to organize the file structure given the HashableInterface implementation. That probably deserves some thought beyond what I suggested.

andkaran commented 10 years ago

An update:

upon rechecking the edits @evantishuk has mentioned. I noticed my stupid mistake. I forgot to comment out

$assets[] = new FileAsset($this->getFullPath($base, 'javascripts'), $filters); in SprocketsRepository so effectively I was running both.

Now upon rechecking. I have 60+ assets which are a mix of coffee-scripts and handlebar templates. I get 10sec with no concat and 3sec with concat.

evantishuk commented 10 years ago

Several things I noticed when testing the Assetic-first caching integration:

First, if you have your CSS manifest looking for, say, pets.scss.css and that file imports child SASS file(s):

@import "_cats.scss";
@import "_dogs.scss";

The cache does not know if _cats.scss or _dogs.scss might have been modified because they're not in the manifest. The work around is to simply update pets.scss.css but the ideal solution would be to scan for SASS imports and check the timestamps on those files as well. Or, perhaps, imports should be avoided and added to the manifest? This is, I think, a minor issue for now.

Second, Assetic allows for a number of possible caching methods https://github.com/kriswallsmith/assetic/tree/master/src/Assetic/Cache (apc, array, file, expiring, etc). This is nice but should these options be exposed through Asset Pipeline?

Third, Assetic could make clearing the cache a challenge. For instance, it's not obvious at all to me if it's feasible or even possible to manually clear the cache that's generated through Assetic. There is a method for removing a cached key (https://github.com/kriswallsmith/assetic/blob/master/src/Assetic/Cache/FilesystemCache.php#L57) but, the function that generates the key is private (https://github.com/kriswallsmith/assetic/blob/master/src/Assetic/Asset/AssetCache.php#L147) and doesn't appear to return the key anywhere outside of that class. Short of mirroring that cache key function within AssetPipeline, I don't see how we could integrate it into the CLI option. I've posted the question (https://github.com/kriswallsmith/assetic/issues/501#issuecomment-25337147) and hopefully I'm just sleep deprived and missing something obvious.

Fourth, does leveraging the caching methods in Assetic possibly change how caching is done elsewhere in this package? Maybe a bigger question is should we explore making a full use of Assetic's tools and possibly refactor a bunch of existing code? Or maybe stop looking at Assetic and just continue to roll our own?

kdocki commented 10 years ago

I'm consolidating this issue into https://github.com/CodeSleeve/asset-pipeline/issues/34 so that I don't have multiple issues open. I am going to spend some time experimenting ways to speed up asset pipeline when you have many assets. I'd like to be able to accommodate at least match Rails speed (it can load ~200 assets in 3 second page refresh on my machine).

CodeSleeve / asset-pipeline

time latency per file #38