gulp-hb icon indicating copy to clipboard operation
gulp-hb copied to clipboard

Performance while watching lots of files

Open janrembold opened this issue 6 years ago • 23 comments

Hi @shannonmoeller,

we experience really bad performance issues while watching lots of hbs templates and partials in some of our larger projects. Especially on windows systems the task runs minutes to complete. I think this is caused by a fresh and complete rebuild of the hbStream on every task execution.

I was thinking about creating the stream only once on startup and updating only changed parts incrementally on file changes, but sadly haven't found a way to manage that.

I think this incremental update approach would boost the performance dramatically. Do you think this is possible somehow?

Thanks for any help in advance!!!

janrembold avatar Mar 08 '18 19:03 janrembold

Sorry for the delay in responding. How many is "lots?" :) This is the first issue I've had reporting performance issues. Way under the hood I'm using require-glob which is backed by node-glob. I've been thinking of updating that to fast-glob, but I'd like to narrow down where the performance issues really are.

Is your project open source?

shannonmoeller avatar Mar 16 '18 15:03 shannonmoeller

Hi @shannonmoeller, I need to count those partials but I think it's 200+. Our projects are not open source but the build framework that uses your gulp task is: https://github.com/biotope/biotope-build

Last week I learned two things:

  1. The worst performance issues occur while switching branches without restarting the watch tasks (I heard about that after I wrote the issue above)
  2. Most of the processing time is consumed by the registerPartial method in handlebars itself

After debugging your code I don't think this is in any case related to your plugin, sorry for buggin you.

But anyway, I think it would still be an interesting feature to cache partials. That would boost the overall performance dramatically. But I think this needs to happen directly inside handlebars package. If you have any ideas how we could get something started, let me know! :)

janrembold avatar Mar 18 '18 19:03 janrembold

You can use a cache of the helpers and partials like this:

const gulp = require('gulp');
const hb = require('gulp-hb');
const rename = require('gulp-rename');

const hbStream = hb({ bustCache: false })
    .helpers('helpers/**/*.js')
    .partials('partials/**/*.js');

function markup() {
    return gulp
        .src('src/posts/**/*.hbs')
        .pipe(hbStream)
        .pipe(rename({ extname: '.html' }))
        .pipe(gulp.dest('dest'));
}

function watchMarkup() {
    return gulp.watch('src/posts/**/*.js', markup);
}

gulp
    .task('markup', markup)
    .task('watch-markup', watchMarkup);

The problem is that if you change any of your partials or helpers you won't get the newest versions when the files are rebuilt by the watch task. I can see how switching between branches without restarting the watch task would be quite the hit to performance.

There might be room to add a feature to gulp-hb to monitor partials and helpers for changes, but that gets really complex really quickly. The current cache makes use of Node.js's internal module caching which would have to be replaced.

shannonmoeller avatar Mar 19 '18 16:03 shannonmoeller

I thought about that too, and now I understand the usage of the bustCache option! But my conclusion was the same as yours. I think we would need some really complex incremental caching logic behind that.

First I thought about using something like https://github.com/gulp-community/gulp-cached that only pipes changed files into handlebars registerPartial function that would need something like an add (if its a new template) or replace (if the template changed) logic.

janrembold avatar Mar 19 '18 20:03 janrembold

@shannonmoeller have you ever tried to add updated partials to the hbStream with bustCache set to false? Something like:

const hbStream = hb({ bustCache: false })
    .partials('partials/partial1.hbs')
    .partials('partials/partial2.hbs');

// update partial1 and call it again
hbStream.partials('partials/partial1.hbs');

If that works we already have the incremental update mechanism :D

janrembold avatar Mar 21 '18 19:03 janrembold

It's possible. I've never tried.

shannonmoeller avatar Mar 21 '18 20:03 shannonmoeller

You might need to do:

hbStream.partials('partials/partial1.hbs', { bustCache: true });

shannonmoeller avatar Mar 21 '18 20:03 shannonmoeller

The real issue in solving this transparently is the dependency tree. If you only want to update changed partials, you likely also only want to update files that make use of that partial, etc. That tree doesn't exist right now.

shannonmoeller avatar Mar 21 '18 20:03 shannonmoeller

I will try that... and tell you if it worked out. That would be a really great performance booster!!!

janrembold avatar Mar 21 '18 20:03 janrembold

We just wrote at the same time :) Maybe we switch to gitter?! But first finalize the thought here....

What I saw during tests the registerPartial was the main performance killer. If that is gone, or reduced to just updating a single file, the process of compiling all root templates should be ok.

Also knowing the dependency tree and updating only the relevant partials would be the perfect solution. But updating and re-using the hbStream would be a major first step!

Thank you already for your feedback!

janrembold avatar Mar 21 '18 20:03 janrembold

Ok, I just tried some things. First I modified the hbStream to be initially filled only once and updated by the watch task:

const hbStream = hb({ bustCache: false })
    .partials('partials/**/*.hbs');
}

gulp.task('hb', () => {
    return gulp
		.src('pages/**/*.hbs')
		.pipe(hbStream)
		.pipe(rename({extname: ".html"}))
		.pipe(gulp.dest(config.global.dev))
}

gulp.task('watch:hb', () => {
    watch(files, config.watch, (vinyl) => {
		hbStream.partials(vinyl.path, { bustCache: true });
		runSequence('hb');
    });
}

This ends with an write after end in "undefined" error on the second run. I think the stream was closed or not closed correctly?!

Second test was the same code with pump syntax. I don't see any errors but the partials are not updated.

gulp.task('static:hb', function (cb) {
	pump([
		gulp.src('pages/**/*.hbs'),
		hbStream,
		rename({extname: ".html"}),
		gulp.dest(config.global.dev)
	], cb);
});

// watch task is the same as above

This just runs but doesn't update the partials inside hbStream :(

janrembold avatar Mar 22 '18 09:03 janrembold

You're right. The stream is getting closed. Does this work?

Edit: Deleted bad code example.

shannonmoeller avatar Mar 22 '18 14:03 shannonmoeller

Actually, that won't work because hbStream is now undefined in the watch task. Doh.

Let me rethink that. :)

shannonmoeller avatar Mar 22 '18 14:03 shannonmoeller

Ok. I think I have a way to do this, but it's kind of annoying.

const gulp = require('gulp');
// import rename, watch, and runSequence
const hb = require('gulp-hb');
const handlebarsWax = require('handlebars-wax');

// Use the same handlebars instance for everything
const handlebars = hb.handlebars;

// Create a new wax wrapper so we can use `.partials()`
const wax = handlebarsWax(handlebars, {
    bustCache: true
});

gulp.task('hb', () => {
    // Go ahead and recreate the stream every time,
    // but use the shared handlebars instance
    const hbStream = hb({ bustCache: false, handlebars })
        .partials('partials/**/*.hbs');

    return gulp
        .src('pages/**/*.hbs')
        .pipe(hbStream)
        .pipe(rename({extname: ".html"}))
        .pipe(gulp.dest(config.global.dev))
});

gulp.task('watch:hb', () => {
    watch(files, config.watch, (vinyl) => {
        // Register changed partials on the shared Handlebars
        // instance with our wax wrapper
        wax.partials(vinyl.path);
        runSequence('hb');
    });
});

This is just a hack to see if this actually solves the performance issues you're seeing. gulp-hb uses handlebars-wax under the hood, and this is just a way to get everything using the same instance of Handlebars every time.

shannonmoeller avatar Mar 22 '18 15:03 shannonmoeller

Also, one case this doesn't solve is the deletion of a partial.

shannonmoeller avatar Mar 22 '18 16:03 shannonmoeller

In your example I don't see any connection between wax and the hbStream, except they are using the same handlebars instance. The code that eats most performance is hb().partials() and that is recreated every time the task executes.

As long as wax (or in the end handlebars itself) is not capable of adding/updating/removing single partials this won't work.

By the way, removing of files can be found out by the vinyl state of the file given by the watch task.

janrembold avatar Mar 22 '18 17:03 janrembold

except they are using the same handlebars instance

Exactly.

eats most performance is hb().partials()

I'll need to look into how I'm compiling the templates.

removing of files can be found out

Found out, yes, but there's no API at present to remove it from the internal Handlebars cache. That would need to be added.

shannonmoeller avatar Mar 22 '18 17:03 shannonmoeller

Hi @shannonmoeller, I just invited you to a minimal demo repository that reflects the idea behind this "caching hbs" challenge: https://github.com/janrembold/handlebars-cache

Hopefully it helps to get it kickstarted somehow...

janrembold avatar Mar 23 '18 12:03 janrembold

Awesome. Thanks!

shannonmoeller avatar Mar 23 '18 15:03 shannonmoeller

I tried to debug into gulp-hb->wax->handlebars and got lost pretty quick :(

So I tried another approach to simply proof what I want to achieve and this PoC works like a charm!!! This is not the gulp'ish way and a very simple demo but it really works great. Please have a look at this branch https://github.com/janrembold/handlebars-cache/tree/precompilation-tests

janrembold avatar Mar 24 '18 10:03 janrembold

The same works fine with handlebars-wax: https://github.com/janrembold/handlebars-cache/tree/wax

janrembold avatar Mar 24 '18 21:03 janrembold

Me again... I tried to refactor the gulp-hb demo (https://github.com/janrembold/handlebars-cache/tree/master) but that doesn't work. The bustCache option has no effect on the compiled template. The resulting template isn't updated.

But honestly I think we don't need the gulp wrapper anymore for this kind of workflow. This is not a real pipeline workflow anymore. I don't see any positive effects from using gulp pipelines over direct node fs calls.

I also think wax.unregisterPartials would be a helpful and necessary extension to your handlebars-wax API: http://handlebarsjs.com/reference.html#base-unregisterPartial

If I find the time and understand your unhookRequire construct, I would like to create a PR if this additional feature would be OK for you?! What do you think?

janrembold avatar Mar 25 '18 08:03 janrembold

Hi @shannonmoeller,

I did lots or research and refactorings during the last weeks and ended up with a complete rewrite of the task based on plain handlebars. If you are curious you can have a look at the task here: https://github.com/biotope/biotope-build/blob/hb-static-performance/tasks/hb2.js

In short, this task uses only a single handlebars instance that is initially filled with all partials and helpers. I also pre-collect and compile all templates and data files and update only single files during runtime with specific watch tasks. After those really tiny updates I compile all templates again because we never know which templates are affected by partial/data changes.

In our large projects this is up to 30% faster than the "easy way" using gulp-hb package.

But I'm really confused about some results that came up with this refactoring. As stated above in an earlier post I found out that the registerPartial function consumes up to 70% of the complete task in gulp-hb.

Within the new refactored plain handlebars task I see something completely different. The registering and compiling of all templates and partials takes only about 16% of the total task execution time. More than 80% of the time is consumed by compiling the static html strings.

So I asked myself if you have found any ways to dramatically speed up hbs compilation in any way. All handlebars options, like assumeObjects or data don't have any effect on the total compilation time. If you had any findings or best practices during your implementation of handlebars-wax or gulp-hb it would be really great to share them.

Thanks in advance!!!

janrembold avatar Apr 17 '18 09:04 janrembold