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

Enable PaRSEC profiling system #227

Closed
wants to merge 2 commits into from

Conversation

therault
Copy link
Contributor

In order to have a working profiling system, the parsec_task_class
of each new TT needs to allocate 2 integers in an array that belongs
to the parsec_taskpool_t. Then, the profiling assumes that the
task class can compute an identifier that helps matching the end
of a task with the beginning, when those do not happen on the same
thread (e.g. for GPU or asynchronous execution). A few more fields
are used by the profiling system that need to be set.

@therault therault marked this pull request as draft March 25, 2022 04:23
@therault
Copy link
Contributor Author

@devreal I added a register_new_tt, but I'm sure we should use the existing infrastructure that keep tracks of the TT lifecycle (world_impl.register_op()). But I didn't want to mess around that without talking with you, so I did it this way for review.

@evaleev @robertjharrison What is our TT creation model? Do we want to authorize multithreaded TT creation? That needs to be protected (and that's not the only thing that will be fragile if multiple threads can create TTs, I believe).

@evaleev
Copy link
Contributor

evaleev commented Mar 25, 2022

In MAD backend TT creation must be done from a single thread, I believe.

@robertjharrison
Copy link
Contributor

robertjharrison commented Mar 25, 2022 via email

@@ -273,6 +282,21 @@ namespace ttg_parsec {
#endif // TTG_USE_USER_TERMDET
}

template <typename keyT, typename output_terminalsT, typename derivedT, typename input_valueTs = ttg::typelist<>>
void register_new_tt(const TT<keyT, output_terminalsT, derivedT, input_valueTs> *t) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be sufficient to take TTBase* here since you only need the name.

@@ -2206,6 +2240,10 @@ namespace ttg_parsec {
}
}

static parsec_key_t make_key(const parsec_taskpool_t *tp, const parsec_assignment_t *as) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand what make_key does... parsec_assignment_t is a struct with one integer member. How is it safe to cast it to a uintptr_t below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.locals is an array of assignment_t with more than 4 of them, so there is room in self.locals to store the pointer-size int. Not clean, but I'm abusing the only space I have to store stack information.

@@ -161,7 +164,12 @@ namespace ttg_parsec {

public:
static constexpr const int PARSEC_TTG_MAX_AM_SIZE = 1024 * 1024;
WorldImpl(int *argc, char **argv[], int ncores) : WorldImplBase(query_comm_size(), query_comm_rank()) {
WorldImpl(int *argc, char **argv[], int ncores) : WorldImplBase(query_comm_size(), query_comm_rank())
#if defined(PARSEC_PROF_TRACE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make that initialization independent of PARSEC_PROF_TRACE but make it an assignment here https://github.com/TESSEorg/ttg/pull/227/files#diff-b16b2b248d6db2366353f63da94be820a8905343d7452acc0639ffcaf3100068R330

@therault
Copy link
Contributor Author

Stale code superseded by PR #222

@therault therault closed this Apr 25, 2022
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

4 participants