Feature/reimplement queries

[koenichiwa]

Current work for #77. Still a work in progress.
It has a logical errror I'm trying to fix. After that I'm looking into combining prev_from and next_after using an enum and some match clauses.

[zslayton]

Based on a quick read:

• I love how much code/indentation this eliminates.
• I love how much this simplifies the flow of control in an otherwise very gnarly looking method.
• I wonder about the performance overhead of creating so many DateTime copies. I notice that DateTime implements Copy, so it's probably not too bad.

[koenichiwa]

I wonder about the performance overhead of creating so many DateTime copies. I notice that DateTime implements Copy, so it's probably not too bad.

At most this will be 7 * 7 copies of a 32 bit integer. DateTime also implements clone copy, so it's can basically get copied bit for bit. I reckon there's a big chance that an compiler can optimize this.
On the other hand, you don't have to set all the values for the query struct anymore

[koenichiwa]

I was looking into merging the functions and making a query function with a direction argument. But it's not looking prettier. I'm starting to doubt if I should merge these two

[koenichiwa]

At most this will be 7 * 7 copies of a 32 bit integer. DateTime also implements clone copy, so it's can basically get copied bit for bit. I reckon there's a big chance that an compiler can optimize this.
On the other hand, you don't have to set all the values for the query struct anymore

Shamefully, it seems that my logic was flawed. I ran a rudimentary benchmark:

fn main() {
    let expression = "0-59 * 0-23 ?/2 1,2-4 ? *";
    let schedule = cron::Schedule::from_str(expression).unwrap();
    let mut moment = Some(Utc.ymd(2017, 6, 15).and_hms(14, 29, 36));
    let current_time = Instant::now();
    let mut schedule_iterator = schedule.after(&moment.unwrap());
    for _ in 0..1000_000 {
        moment = schedule_iterator.next();
    }
    println!("Last moment: {:?} Time of exectution: {}", moment, current_time.elapsed().as_secs());
}

And for my branch this was the output: Last moment: Some(2018-01-23T13:46:39Z) Time of exectution: 184
For the original branch this was the output: Last moment: Some(2018-01-23T13:46:39Z) Time of exectution: 26

That means, back to the drawing board for me.

[zslayton]

Thanks for taking the time to benchmark it, I definitely wasn't expecting a ~7x slowdown.

Shamefully, it seems that my logic was flawed.

For what it's worth, I totally bought your reasoning. I didn't realize DateTime values were as small as they are, and would have expected the compiler to eliminate most of the costs I was worried about.

[koenichiwa]

I think because it needs to retain 7 copies of each datetime due to iter::repeat. I might be able to find a solution with raw ordinals, but I'm putting that thought in the vault for now.

If someone else feels the need to take on this challenge in the meantime, they're more than welcome to

[zslayton]

Closing as part of a cleanup, feel free to reopen.