Trilinos icon indicating copy to clipboard operation
Trilinos copied to clipboard

Intrepid2: Eliminate Extraneous "zeroView_" allocations during Data construction

Open CamelliaDPG opened this issue 3 years ago • 5 comments

Enhancement

@trilinos/intrepid2

The cost of Data construction is dominated in most cases by an allocation of a DynRankView with a single zero entry in it. This is used by Data to allow returns of references to zero in a way that is consistent with returns of references to data that is explicitly stored by the Data object. There is, however, no reason that each Data object must have its own copy of the zero View.

The cost of this can be significant; after converting Camellia to use the new-style Intrepid2 Basis allocation mechanisms, as well as Data in other places, profiling suggests that approximately 20% of the runtime of the tests is spent in this particular allocation.

I have a draft implementation that constructs the appropriate DynRankView statically; on Data construction, this sets the zeroView_ member to the statically allocated DynRankView. I have not yet tested this at all (waiting for it to build on my laptop at the moment). It will be important to confirm that this works under CUDA; I'm a little vague on CUDA's rules for static allocations -- I think what I've done will work, but can't be entirely sure without testing it.

CamelliaDPG avatar Aug 04 '21 15:08 CamelliaDPG

@kyungjoo-kim @mperego With my draft implementation, I'm getting an error on Kokkos::finalize():

Kokkos allocation "zero" is being deallocated after Kokkos::finalize was called

So I need to do something to deallocate the view prior to Kokkos::finalize. In Camellia to solve this, I implemented by own Camellia::finalize() which users should call, and that does the appropriate cleanup. Do we have an existing solution to this in Intrepid2? I expect we do, since I know we have some static data; could one of you point me to it?

CamelliaDPG avatar Aug 04 '21 16:08 CamelliaDPG

@CamelliaDPG I happened to catch this when skimming email updates, for static data you can register a finalize hook with a lambda to deallocate the View, then when Kokkos::finalize is called it will trigger the finalize hook to deallocate, e.g.

https://github.com/trilinos/Trilinos/blob/ee32a982416676b96c7177f93fc222f9afb762b4/packages/intrepid2/src/Cell/Intrepid2_CellDataDef.hpp#L253-L269

ndellingwood avatar Aug 04 '21 16:08 ndellingwood

@ndellingwood Thanks; that's exactly what I was after!

CamelliaDPG avatar Aug 04 '21 16:08 CamelliaDPG

right!

mperego avatar Aug 04 '21 16:08 mperego

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity. If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label. If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE. If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

github-actions[bot] avatar Aug 06 '22 12:08 github-actions[bot]

This issue was closed due to inactivity for 395 days.

github-actions[bot] avatar Sep 07 '22 12:09 github-actions[bot]