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

Calculating VAVAILABILITY #242

Merged
merged 12 commits into from
Jul 17, 2015
Merged

Calculating VAVAILABILITY #242

merged 12 commits into from
Jul 17, 2015

Conversation

evert
Copy link
Member

@evert evert commented Jul 8, 2015

This PR is a work in progress to add VAVAILABILITY calculations to sabre/vobject.

VAVAILABILITY is currently largely useful in when generating freebusy reports, so this is also where this needs to be fixed. It makes the calculations a lot more complex, so it's also requiring some refactoring in the freebusy generator.

Todo:

  • Reintegrate the new FreeBusyData object back into FreeBusyGenerator. It's kind of nice separate, as it is, but I'm afraid there's little use for it outside of freebusy calculations. By merging it back into the class we keep a cleaner namespace.
  • Do the actual VAVAILABILITY expansion.

Benchmark

Old:

__global__  |||||||||||||||||||||||||||||||||||||||||||||||||||| 15891ms, 100.0%
parse       |                                                      358ms,   2.3%
fb          |||||||||||||||||||||||||||||||||||||||||||||||||||  15533ms,  97.7%

peak memory usage: 14M
current memory usage: 12M

New:

__global__  |||||||||||||||||||||||||||||||||||||||||||||||||||| 22741ms, 100.0%
parse       |                                                      308ms,   1.4%
fb          |||||||||||||||||||||||||||||||||||||||||||||||||||  22433ms,  98.6%

peak memory usage: 14M

Unfortunately it seems that the new system is quite a bit slower. We'll have to figure out how important it is to fix this, and if there's some easy wins to be made.

@evert evert self-assigned this Jul 8, 2015
@evert evert added this to the 4.0 milestone Jul 8, 2015
@evert
Copy link
Member Author

evert commented Jul 17, 2015

Not gonna bother with making it faster. It's much better this way and we already got major speedups in the 4.x branch, I think it's within acceptable limits.

I'm also not reintegrating FreeBusyData into FreeBusyGenerator. It's a bit unfortunate to have a separate class, but it does make things a lot more clear.

So when I fixed the broken tests, this is ready for merging.

@evert evert removed the in-progress label Jul 17, 2015
evert added a commit that referenced this pull request Jul 17, 2015
@evert evert merged commit d498e14 into master Jul 17, 2015
@evert evert deleted the vavailablity-calculation branch July 17, 2015 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant