Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render never runs within synchronous loop #1

Closed
jimbuck opened this issue Jan 14, 2016 · 6 comments
Closed

Render never runs within synchronous loop #1

jimbuck opened this issue Jan 14, 2016 · 6 comments
Assignees
Milestone

Comments

@jimbuck
Copy link

jimbuck commented Jan 14, 2016

Since the internal render function is only called on the tick of a setInterval it will never get rendered if looping through a set of synchronous actions.

A better approach would be to to keep track of when the bar was rendered last, and only attempt to re-render when update is called. That would give the same optimization as re-rendering when there is a change but also works with synchronous code.

@AndiDittrich AndiDittrich added this to the v1.1 milestone Jan 14, 2016
@AndiDittrich
Copy link
Member

Dear Jimmy,

do you have an example for such blocking synchronous code ? i thought the setInterval callback should by executed by the v8 engine when the update() function is called, because it's non-blocking in this moment ?!

@jimbuck
Copy link
Author

jimbuck commented Jan 19, 2016

Below is a small example, based on the usage details from your readme.

var _progress = require('cli-progress');

// create a new progress bar instance
var bar1 = new _progress.Bar();

// Some set of data
var data = [];

// start the progress bar with a total value of 200 and start value of 0
bar1.start(data.length, 0);

for (var i = 0; i < data.length; i++)
{
  // Some operation that uses the data (synchronously)
  useData(data[i]);

  // update the current value in your application..
  bar1.update(1);
}

// stop the progress bar
bar1.stop();

No updates will happen until after the data is iterated. Take a look at this post by John Resig for nitty gritty, but the basic idea is that synchronous operations (iterating sequentially) does not allow a timeout or interval to execute a callback.

@AndiDittrich AndiDittrich self-assigned this Jan 22, 2016
@AndiDittrich
Copy link
Member

Dear Jimmy,

thanks for your hint. the whole for-loop is executed as "synchronous" block, therefore the interval queue is not triggered. i've created a simple testcase to reproduce the issue:

var _progress = require('./main');
var limit = 2000;

// long running, recursive function (blocking operation)
function fibonacci(n) {
    if (n < 2) {
        return 1;
    }else {
        return fibonacci(n - 2) + fibonacci(n - 1);
    }
}

// create new progress bar using default values
var b1 = new _progress.Bar();
b1.start(limit, 0);

for (var i = 0; i < limit; i++) {
    var r = fibonacci(i);
    console.log("Step ", i, " #", r);
    b1.update(i);
}

b1.stop();

i will fix this problem asap.

@jimbuck
Copy link
Author

jimbuck commented Jan 22, 2016

I actually came up with a nice workaround, but it could only be done with a different progress bar library.

As long as you have a stop or done command, you actually don't need a timer at all. The key thing is to watch the time whenever tick is called. If too little time has passed, then add the tick amount to a property and wait until the next call. If enough time has passed, re-render taking into account the tick sum to update.

Here is a simplified version of how I made it work with a very simple library:

const Bar = require('progress');

const TICK_DELAY = 600; // Can be calculated based on FPS (Should just be 1000/fps).

class ProgressBar {
    constructor(total) {
        // Create a simple progress bar, that
        this._bar = new Bar(' :bar :percent | ETA: :etas'), {
            total,
            complete: ' ',
            incomplete: '█',
            width: 40
        });

        // Let's keep track of the total ticks that must be applied.
        this.tickAmount = 0;
        // Let's also store the millisecond timestamp from the last call.
        this.prevTick = 0;
    }

    tick(amount) {
        // Auto-increment by 1 if not specified...        
        if(typeof amount !== 'number'){
            amount = 1;
        }       

        // Add the amount to the total ticks that must be applied...
        this.tickAmount += amount;

        // Get the time difference, in milliseconds...
        let now = Date.now();
        let diff = now - this.prevTick;

        // If the time since the last successful tick is more than the threshold...
        if(diff > TICK_DELAY) {
            // Tick the full expected amount and reset the variables...
            this._bar.tick(this.tickAmount);
            this.prevTick = now;
            this.tickAmount = 0;
        }
    }

    complete(){
        // If there is any unapplied ticks, apply them now.
        if(this.tickAmount){
            this._bar.tick(this.tickAmount);
        }
    }
}

module.exports = ProgressBar;

The best part of this is that for rapid tick calls it will throttle them easily. But for long slow calls it will only update when you tell it to, instead of on a repeating interval (think some operation that takes 10 seconds).

@AndiDittrich
Copy link
Member

this solution might work for long running operation but causes issues related to the ETA calculation.
in case the tick() function is called in the following way:

throttle time: 25ms

Elapsed Time Update-Value Rendering Status
10ms 5 updated
20ms 20 rejected
50ms 30 updated
80ms 50 updated
100ms 290 rejected
4s 408 updated

the update on 100ms is rejected and no update will happen until the next call. to avoid such problem, you have to run a timer in the background which will trigger the re-rendering (maybe on 2*throttle_time ?)

i know, this example is very theoretical, but i think a new version should also work under such conditions ?

@AndiDittrich
Copy link
Member

I've added synchronous operation support to the current "v1.1.0" release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants