You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
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).
functiondoInBackground(items,processItem,maxBlockingTime,idleTime){// Argument defaultsmaxBlockingTime=maxBlockingTime||20;idleTime=idleTime||30;varsliceStartTime=(newDate()).getTime();returndoSequentially(items,function(item,i){varresult=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((newDate()).getTime()-sliceStartTime>=maxBlockingTime){window.setTimeout(function(){sliceStartTime=(newDate()).getTime();result.resolve();},idleTime);}else{result.resolve();}returnresult;},false);}
What do you think? Does this remain flexible enough for your (and similar) needs?
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?
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).
functiondoInBackground(items,processItem,maxBlockingTime,idleTime){// Argument defaultsmaxBlockingTime=maxBlockingTime||20;idleTime=idleTime||30;varsliceStartTime=(newDate()).getTime();doSequentially(items,function(item,i){varresult=new$.Deferred();processItem(item,i);// If our time slice is done, pause before letting the nextiteminthesequencebegin// processing. If we still have more time left, let the nextitemstartimmediately.if((newDate()).getTime()-sliceStartTime>=maxBlockingTime){window.setTimeout(function(){sliceStartTime=(newDate()).getTime();result.resolve();},idleTime);}else{result.resolve();}returnresult;},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: #1009 (comment)
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.
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.