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

[CLOSED] Add process in background options to doSequentially #1003

Open
core-ai-bot opened this issue Aug 29, 2021 · 5 comments
Open

[CLOSED] Add process in background options to doSequentially #1003

core-ai-bot opened this issue Aug 29, 2021 · 5 comments

Comments

@core-ai-bot
Copy link
Member

Issue by jhatwich
Thursday Jun 07, 2012 at 12:57 GMT
Originally opened as adobe/brackets#1009


Do sequentially can now be used to process a list of tasks in time
slices so long-running processes don't block up the UI.

I ran into the need for this in creating a related files extension.


jhatwich included the following code: https://github.com/adobe/brackets/pull/1009/commits

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Saturday Jun 09, 2012 at 00:30 GMT


Very interesting! A few questions/comments:

  • It feels like this makes doSequentially() serve two very distinct purposes: sequencing and aggregating asynchronous tasks (which happen mainly off the JS thread) plus running slices of synchronous processing (which happen entirely on the JS thread). I'm wondering if it would be cleaner to recast this as a new API that sits on top of doSequentially(), keeping doSequentially() simpler and more narrowly scoped.
  • Could you talk a bit about the use cases for sliceNow = true vs. false? In what situations would you want to delay processing for 30ms?
  • Should we expose TIME_SLICE and TIME_YIELD as optional arguments? It seems like the caller might want to finesse their values depending on use case (e.g. a long-running modal operation that only wants to unblock long enough to repaint a progress bar might prefer bigger slices, while an operation that runs continuously while the user is typing might want shorter slices or longer idle gaps).

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Saturday Jun 09, 2012 at 00:35 GMT


Here's an example of the notion of building this API on top of doSequentially() instead of integrated into it. (I haven't tested this code at all yet, so caveat emptor).

    function doInBackground(items, processItem, maxBlockingTime, idleTime) {
        // Argument defaults
        maxBlockingTime = maxBlockingTime || 20;
        idleTime = idleTime || 30;

        var sliceStartTime = (new Date()).getTime();

        return doSequentially(items, function (item, i) {
            var result = new $.Deferred();

            processItem(item, i);

            // If our time slice is done, pause before letting the next item in the sequence begin
            // processing. If we still have more time left, let the next item start immediately.
            if ((new Date()).getTime() - sliceStartTime >= maxBlockingTime) {
                window.setTimeout(function () {
                    sliceStartTime = (new Date()).getTime();
                    result.resolve();
                }, idleTime);
            } else {
                result.resolve();
            }

            return result;
        }, false);
    }

What do you think? Does this remain flexible enough for your (and similar) needs?

@core-ai-bot
Copy link
Member Author

Comment by jhatwich
Saturday Jun 09, 2012 at 01:00 GMT


Making it a separate API would work fine for my needs - the optional
parameters thing doesn't bother me personally, but I can see how it might
make it more confusing to digest the API. I added it to doSequentially
because the background processing behavior is what I expected that function
to do in the first place :) Maybe "sequenceInBackground" or
"doSequentiallyInBackground"?

Exposing idleTime and maxBlockingTime is a good idea too.

The sliceNow option allows you to try to get something done sync, but defer
it if it takes too long. I'm not using that option, but as a utility it
seemed like it could be useful in some situations. I'm using it with
sliceNow false to make sure the operation I'm scheduling happens "in the
background" - for related files I didn't want to chance a performance hit
on load since there is already a lot going on. It might be better as a
milliseconds value - how long to delay the first slice (defaulting to 0)
"delayStartTime" perhaps?

  • Josh

On Fri, Jun 8, 2012 at 5:35 PM, Peter Flynn <
reply@reply.github.com

wrote:

Here's an example of the notion of building this API on top of
doSequentially() instead of integrated into it. (I haven't tested this
code at all yet, so caveat emptor).

   function doInBackground(items, processItem, maxBlockingTime, idleTime) {
       // Argument defaults
       maxBlockingTime = maxBlockingTime || 20;
       idleTime = idleTime || 30;

       var sliceStartTime = (new Date()).getTime();

       doSequentially(items, function (item, i) {
           var result = new $.Deferred();

           processItem(item, i);

           // If our time slice is done, pause before letting the next
item in the sequence begin
           // processing. If we still have more time left, let the next
item start immediately.
           if ((new Date()).getTime() - sliceStartTime >= maxBlockingTime)
{
               window.setTimeout(function () {
                   sliceStartTime = (new Date()).getTime();
                   result.resolve();
               }, idleTime);
           } else {
               result.resolve();
           }

           return result;
       }, false);
   }

What do you think? Does this remain flexible enough for your (and
similar) needs?


Reply to this email directly or view it on GitHub:
adobe/brackets#1009 (comment)

@core-ai-bot
Copy link
Member Author

Comment by jhatwich
Saturday Jun 30, 2012 at 12:29 GMT


Sorry for the long delay. I've finally applied the refactoring to Async based on your code and it seem to be working great. I left delayStartTime out for now since I'm not using it anyway.

  • Josh

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Jul 27, 2012 at 23:48 GMT


@jhatwich: Sorry for my long delay as well :-) It looks good to me -- merging now.

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

No branches or pull requests

1 participant