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.

Sign up for GitHub

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

Jump to bottom

Replace buffer tracking with serialization inspection #301

Merged
merged 41 commits into from
Jan 30, 2025
Merged

Replace buffer tracking with serialization inspection #301

merged 41 commits into from
Jan 30, 2025

Conversation

devreal
Copy link
Contributor

@devreal devreal commented Nov 8, 2024

We have been tracking the buffers of structures by having the buffer register itself with some metadata. This is clumsy and error-prone (no support for nesting, for example). Now we use a specialized archive to inspect all members of a value and apply some functor to each buffer (or its internal parsec data).

This is also the first step towards a structure derived from parsec_data_copy_t to better manage the life-time of the host copy. We now only allow shared_ptr for non-owning buffers so some adjustments must be made by users.

@devreal devreal marked this pull request as draft November 8, 2024 02:11
@devreal devreal marked this pull request as ready for review November 13, 2024 20:11
@devreal
Copy link
Contributor Author

devreal commented Nov 13, 2024

I think this is ready. However, we can only achieve such introspection using the madness archive. With boost, we would have to specialize traits on the inspector archive, which is not possible because the inspector archive is templated on the callable. Even if we were to type-erase the callable into std::function we'd still have to pick the value and allocator type for the Buffer that gets passed in. Not possible, because objects can hold different types of buffers at once.

I don't think this is an issue. Moving from boost to madness serialization is not hard and users have to touch their data structures anyway to integrate buffers. More reliance on madness though, at least for the time being.

@devreal devreal marked this pull request as draft November 13, 2024 20:41
@devreal
Copy link
Contributor Author

devreal commented Nov 13, 2024 •
edited
Loading

Nevermind, not ready yet. This requires ICLDisco/parsec#695 to land in PaRSEC. Reverted back to draft.

@devreal devreal force-pushed the serialize-buffer-query branch 3 times, most recently from d84ae7b to 99dd848 Compare November 15, 2024 17:41
@devreal devreal force-pushed the serialize-buffer-query branch from da4545c to f23639d Compare November 25, 2024 22:19
devreal added 19 commits December 20, 2024 15:27
@devreal
Replace buffer tracking with serialization inspection
a84f920 
We have been tracking the buffers of structures by having the buffer
register itself with some metadata. This is clumsy and error-prone
(no support for nesting, for example). Now we use a specialized
archive to inspect all members of a value and apply some functor
to each buffer (or its internal parsec data).

This is also the first step towards a structure derived from
parsec_data_copy_t to better manage the life-time of the host copy.
We now only allow shared_ptr for non-owning buffers so some adjustments
must be made by users.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Use parsec_data_discard to safely destroy parsec_data_t
7db597c 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Restrict calls to buffer_apply() to serializable types
ae656c4 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
static_assert that device task output value types are serializable
13b6aa6 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Remove static assert
3cce7a8 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Attempt at a boost serialization frontend for buffer_apply
356ade0 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Using ttg::Buffer requires MADNESS serialization
099baef 
We cannot use boost serialization to inspect objects for the
buffers they contain because boost archives cannot be templated properly
and need explicit instantiations for its archives, which is impossible
for the BufferInspectorArchive. We just rely on madness from now on.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Fix usage of smart-pointer for buffers
079b694 
For now only allow T[] on smart pointers passed to buffers.
We may want to support both T and T[] on ttg::Buffer
so we can distinguish the two later.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Bring back Buffer::reset(n) needed for serialization
659746e 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Remove exception from empty_allocator::deallocate()
42d041f 
The data copy will call deallocate no matter what, so be graceful.
We will still catch cases where we try to allocate through
the empty allocator.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Fix serialization and remove split metadata from MatrixTile
1295bac 
We can now inspect the buffers of the tile to extract its memory regions.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Rename BufferInspectorArchive to BufferVisitorArchive
bcde59a 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Bump PaRSEC commit sha to point to a branch with parsec_data_discard()
1bf4b11 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Fix usage of Allocator in MatrixTile
671024f 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Bring back scopes on Buffer construction
0429c7e 
This allows us to mark buffers as allocate-only. SyncIn is the default.
Ignored in the MANDESS backend.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Buffer: handle constness in connection with smart pointers
281fec3 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Add missing parsec_data.h to install list
1effdc1 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Buffer: in-place construct data_copy_type in construct callback
5e7d765 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Protect parsec data buffer inspection against empty buffers
04b67fd 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal devreal force-pushed the serialize-buffer-query branch from f23639d to 04b67fd Compare December 20, 2024 20:57
@devreal devreal marked this pull request as ready for review December 20, 2024 21:00
@devreal
Use smart pointer for buffer in device chain benchmark
03cf233 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Another attempt at fixing CRTP usage in device map setter
bf86552 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Fix chain device test by removing the non-move path
32c8200 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
SPMM: pass shared_ptr to buffer
36a0d23 
Extract the shared ptr from the mohndl and tie the shared_ptr<T[]>
to the life-time of the mohndl shared ptr.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Merge branch 'master' into serialize-buffer-query
86f6124
@devreal devreal mentioned this pull request Jan 6, 2025
Merged
devreal and others added 11 commits January 14, 2025 19:08
@devreal
Merge branch 'master' into serialize-buffer-query
2b8fce9
@devreal
Default to MADNESS serialization with PaRSEC backend
cf0347c 
Boost has little use and we currently need the madness serialization
for GPU support.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
SPMM: Add madness serialization to DeviceTensor
19f06b4 
Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Comment out Boost::serialization dependency
65a61f0 
It's all broken. All of it.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Condition serialization_boost-parsec on TTG_PARSEC_USE_BOOST_SERIALIZ…
75b8a49 
…ATION

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Don't install boost from apt/brew
317bd7c 
The version from apt is 1.74 which is too old so we end up
building our own anyway.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Boost::type_index is required by the PaRSEC backend
8df7142 
Signed-off-by: Joseph Schuchart <[email protected]>
@evaleev
boost_iostreams install fails unless it's built via dependence on a p…
4ecf4e4 
…revious target, exclude it from the build unless TTG_PARSEC_USE_BOOST_SERIALIZATION=ON
@devreal
Revert "Comment out Boost::serialization dependency"
8186846 
This reverts commit 65a61f0.
@devreal
CI: don't specify C++ standard
b440287 
We should specify a standard ourselves independent of CI.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Remove use of Boost::typeindex
87d0055 
The detection is flaky and the benefit little.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal devreal force-pushed the serialize-buffer-query branch from 5ebbc79 to 8a2ec53 Compare January 23, 2025 04:05
@devreal
Copy link
Contributor Author

devreal commented Jan 23, 2025

CI is passing now. Please review :)

@devreal devreal requested review from evaleev and therault January 23, 2025 15:07
@devreal
More fun with Boost serialization
4e6943b 
Don't even bother trying to create a target if
!TTG_PARSEC_USE_BOOST_SERIALIZATION

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal devreal force-pushed the serialize-buffer-query branch from 8a2ec53 to 4e6943b Compare January 24, 2025 18:15
@devreal
Look for CUDA 12, not 12.6
914cc3d 
NVIDIA released CUDA 12.8 which is now the default.

Signed-off-by: Joseph Schuchart <[email protected]>
@devreal
Copy link
Contributor Author

devreal commented Jan 27, 2025

CI is passing again. @evaleev let's get this in before something else breaks :)

@devreal devreal force-pushed the serialize-buffer-query branch from 4fb04da to b502ac8 Compare January 29, 2025 20:40
@devreal devreal merged commit 1e3c28d into TESSEorg:master Jan 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abouteiller abouteiller abouteiller approved these changes

@evaleev evaleev Awaiting requested review from evaleev

@therault therault Awaiting requested review from therault

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@devreal @abouteiller @evaleev