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

various coordinate performance improvements #258

Merged
merged 6 commits into from
Oct 27, 2015

Conversation

juliantaylor
Copy link
Contributor

coordinate transformations in casa are spending most of their time allocating memory instead of actually computing. This set of patches improves this a situation a bit via a few small changes, see the commits for details.

The largest change is using a small Vector cache to avoid creating new Vectors for each MVPosition and Euler temporary object that are created en mass in the conversion code.
This cache could in theory be moved lower into the stack, e.g. in the allocator itself and the Array object would need to use placement new's to create its Block object, though this is quite tricky to get right so I opted for the simpler change of just caching the objects that are problematic.
A disadvantage is that the small cache is never free'd so it would show up in valgrind, but that can be handled with a suppression file.

avoids creating new Unit in often called MVEpoch::getTime()
Avoid creating a temporary MVPosition object in the 3x3 matrix-vector
product by using temporary stack variables.
MVPosition are often created as temporaries where the allocation
overhead of the size 3 vector is large. To reduce this add a very simple
cache for these vectors that holds a couple already allocated vectors.
Euler objects are often created as temporaries where the allocation
overhead of the size 3 vectors is large. To reduce this add a very simple
cache for these vectors that holds a couple already allocated vectors.
the compiler cannot remove then as it must assume memory aliasing.
@juliantaylor juliantaylor force-pushed the perf-improvements branch 2 times, most recently from 9a24a2e to 72c0e23 Compare October 27, 2015 11:57
Returns derivative of MeasTable::posArg and aber*Arg Polynomial,
which is also computed only once and the cached. Avoids unnecessary
Polynomial creations in some places which involved several small Array
allocations.
dpetry added a commit that referenced this pull request Oct 27, 2015
various coordinate performance improvements
@dpetry dpetry merged commit e7e765e into casacore:master Oct 27, 2015
@dpetry
Copy link
Contributor

dpetry commented Oct 27, 2015

This looks great, Julian. Thanks.
The unit tests are quite detailed. So I think there is little risk in these changes.
But we should keep an eye on test failures in the CASA regression tests in the next few days.

@juliantaylor
Copy link
Contributor Author

I run the casa regression tests on casacore PRs already, though I didn't find the time to integrate the result reporting with github yet.
This branch tested fine (but there is an unrelated failure in test_regrid since earlier today)

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

Successfully merging this pull request may close these issues.

None yet

2 participants