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

Make filteredRows rate-limited (throttled) #19

Open
ohadschn opened this issue Dec 20, 2014 · 7 comments
Open

Make filteredRows rate-limited (throttled) #19

ohadschn opened this issue Dec 20, 2014 · 7 comments
Milestone

Comments

@ohadschn
Copy link
Contributor

Binding an input's textInput to the table's filter property is very useful for incremental filtering. However, it may result in unnecessary processing of intermediate user input.

For example, suppose you are bound to a list of 10K city names, and the user type in "Tokyo". On each keystroke ('T', 'o', 'k', and so forth) , the filter is re-evaluated which can be costly, especially on weaker machines.

In such cases it may be desirable to throttle changes to the filteredRows computed property. Fortunately, KO makes this extremely easy and it boils down to a single extend call. The desired throttling interval could be provided in the DataTable constructor (if not specified, no throttling will take place).

Deprecated API (for older KO versions): http://knockoutjs.com/documentation/throttle-extender.html
Current API: http://knockoutjs.com/documentation/rateLimit-observable.html

@cmbankester
Copy link
Contributor

I've never thought about rate-limiting the filteredRows observable itself, but that's an interesting idea. Currently, in our production applications, if we have a large dataset and want incremental filtering, we create a secondary observable (say, inputFilter) and throttle/rate-limit it, using it to write to the datatable's filter. Rate-limiting the underlying filteredRows observable would certainly make incremental changes easier to implement for the user.

@cmbankester cmbankester added this to the 0.6.0 milestone Dec 20, 2014
@ohadschn
Copy link
Contributor Author

That is a clever workaround! However, it won't handle row manipulation scenarios. For example, suppose the 10K city names are additional names you need to add to an existing repository. If the table is sorted, calling addRecord N times in a tight loop will result in N sort operations performed on the array, instead of one in the throttled case (even if the rate limit is zero).

@cmbankester
Copy link
Contributor

That's a really great point. I'll get that added in whenever I have a chance.

@cmbankester cmbankester modified the milestones: 1.0.0, 0.6.0 Jan 21, 2015
@cmbankester
Copy link
Contributor

TLDR, I propose the following:

  • use .extend({rateLimit: 0}) for filteredRows
  • use .extend({rateLimit: this.options.filterRateLimit}) for filter

There is an issue with rate-limiting the filteredRows computed, viz. that if the filter is set high (for instance, high enough to feasibly handle the Tokyo problem detailed above), it then causes a delay equal to the rate-limit time when changing the other dependencies of filteredRows (e.g. sorting a field, thereby changing the sortField observables value).

We can, however, use .extend({rateLimit: 0}) to handle the tight loop problem, as detailed here (at the end of Example 3: Avoiding multiple Ajax requests).

I also propose that we do use a rateLimit extender for the filter observable, but perhaps we should have it be configurable. That way, if the dataset is small, the user can choose to use a small (even 0) value, and if the dataset is large, the user can adjust appropriately.

@ohadschn
Copy link
Contributor Author

Good thinking!

Instead of rate-limiting the fileteredRows observable though, perhaps the rows observable should be rate-limited? That will solve the tight loop issue while still preserving immediate sorting changes (not that I expect much of a difference between "immediate" and "rate-limit 0" in non-programmatic scenarios). As a bonus, it will prevent the redundant (albeit short) recalculations of rowAttributeMap too.

Also note that rateLimit was introduced in KO 3.1.0. I don't mind this dependency, just pointing it out in case you do (in which case throttle can be used for previous versions).

@cmbankester
Copy link
Contributor

I'm inclined to think that rate-limiting the filteredRows observable is still the better option. Consider a (obviously contrived) example of a synchronous loop of calls to the toggleSort observable. We would then need to also rate-limit toggleSort. Rate-limiting filteredRows attacks the problem at the source and alleviates the need to rate-limit any other observables (aside from filter, for obvious reasons).

@ohadschn
Copy link
Contributor Author

In that case perhaps rate-limit both? I admit it's not that important though, either or both will work well in most real-world scenarios (I still give rows the edge for plausibility though ;) )

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

2 participants