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

Performance issue, It could work x3 times better! #23

Closed
nikolay-zakharov opened this issue Aug 3, 2012 · 2 comments
Closed

Performance issue, It could work x3 times better! #23

nikolay-zakharov opened this issue Aug 3, 2012 · 2 comments
Labels

Comments

@nikolay-zakharov
Copy link
Contributor

Hi, I've found a performance bottleneck:

Script version: 1.6.1
Problems are found at lines: 105-107 and 109-111

Original code (lines 105-107):

    $this
        .datepicker("option", "minDate", min_date)
        .datepicker("option", "maxDate", max_date);

Original code (lines 109-111):

    $this
        .datepicker("option", "minDate", this.multiDatesPicker.minDate)
        .datepicker("option", "maxDate", this.multiDatesPicker.maxDate);

This is a bad thing. Each call of $this.datepicker('option', 'min/maxDate', ...) makes datepicker and multiDatesPicker to execute onBeforeShow event listeners for each of the days you have visible in your calendar. So in my 1-year calendar (12*35 days ~ 420 days) single attempt to select a day executes onBeforeShow for 3 times for each day! And it takes about 350 ms x 3 times ~ 1.05 sec for stucking. But if you apply my fix (it is available below), you'll be forced to wait 3 times less before you can click another date.

My suggestion is:
It's better to modify those lines to get them this like:

My solution for lines 105-107

    if(methods.compareDates($this.datepicker("option", "minDate"), min_date) !== 0) 
        $this.datepicker("option", "minDate", min_date)
    if(methods.compareDates($this.datepicker("option", "maxDate"), max_date) !== 0) 
        $this.datepicker("option", "maxDate", max_date)

My solution for lines 109-111

    if(methods.compareDates($this.datepicker("option", "minDate"), this.multiDatesPicker.minDate) !== 0)
        $this.datepicker("option", "minDate", this.multiDatesPicker.minDate);
    if(methods.compareDates($this.datepicker("option", "maxDate"), this.multiDatesPicker.maxDate) !== 0)
        $this.datepicker("option", "maxDate", this.multiDatesPicker.maxDate);

It improves performance of onSelect event for 3 times with the same functionality.

Thanks for this great plugin!

@minitech
Copy link

You should probably submit a pull request...

nikolay-zakharov added a commit to nikolay-zakharov/Multiple-Dates-Picker-for-jQuery-UI that referenced this issue Aug 18, 2012
ghost pushed a commit that referenced this issue Aug 25, 2012
Fixed issue #23, improves onSelect performance
@ghost
Copy link

ghost commented Aug 25, 2012

Pull request merged, thanks for the help.

@ghost ghost closed this as completed Aug 25, 2012
ghost pushed a commit that referenced this issue Aug 25, 2012
Reverted Issue #23 fix code. Turns out that it broke certain parts of the onSelect function. Will refork later.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants