Functions to change taskq wait policy #294
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
evaleev
requested changes
Jul 9, 2021
src/TiledArray/tiledarray.cpp
Outdated
| @@ -127,3 +128,16 @@ void TiledArray::ta_abort(const std::string &m) { | |||
| std::cerr << m << std::endl; | |||
| ta_abort(); | |||
| } | |||
|
|
|||
|
|
|||
| void TiledArray::set_taskq_wait_busy() { | |||
There was a problem hiding this comment.
these really should be free functions in madness namespace, I think ... fine otherwise
|
I wanted to hide madness from mpqc proper, st if taskq changes Only TA is
affected
…On Jul 9, 2021 11:40 AM, "Eduard Valeyev" ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In src/TiledArray/tiledarray.cpp
<#294 (comment)>
:
> @@ -127,3 +128,16 @@ void TiledArray::ta_abort(const std::string &m) {
std::cerr << m << std::endl;
ta_abort();
}
+
+
+void TiledArray::set_taskq_wait_busy() {
these really should be free functions in madness namespace, I think ...
fine otherwise
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#294 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABH62KBSWXMZ447KSQKX6VDTW4C6RANCNFSM5AC23DXA>
.
|
|
NWX plan to use MADNESS runtime directly so they will need to be able to do
this also ... + only MADNESS knows how its taskq is implemented, the
default is to use threads, but it can also use TBB (-DENABLE_TBB=ON) and
PaRSEC (-DENABLE_PARSEC=ON), ideally we can abstract out what needs to
happen inside MADNESS itself
On Fri, Jul 9, 2021 at 12:56 PM Andrey Asadchev ***@***.***>
wrote:
… I wanted to hide madness from mpqc proper, st if taskq changes Only TA is
affected
On Jul 9, 2021 11:40 AM, "Eduard Valeyev" ***@***.***> wrote:
> ***@***.**** requested changes on this pull request.
> ------------------------------
>
> In src/TiledArray/tiledarray.cpp
> <
#294 (comment)>
> :
>
> > @@ -127,3 +128,16 @@ void TiledArray::ta_abort(const std::string &m) {
> std::cerr << m << std::endl;
> ta_abort();
> }
> +
> +
> +void TiledArray::set_taskq_wait_busy() {
>
> these really should be free functions in madness namespace, I think ...
> fine otherwise
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <
#294 (review)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/ABH62KBSWXMZ447KSQKX6VDTW4C6RANCNFSM5AC23DXA
>
> .
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQXIZ42EWP4KJUP3XCGPO3TW4S5DANCNFSM5AC23DXA>
.
--
web: valeyev.net
|
|
There are functions in madness as well, static in ConditionVariable.
Reason they there is because they affect all CVs not just taskq
…On Jul 9, 2021 2:08 PM, "Eduard Valeyev" ***@***.***> wrote:
NWX plan to use MADNESS runtime directly so they will need to be able to do
this also ... + only MADNESS knows how its taskq is implemented, the
default is to use threads, but it can also use TBB (-DENABLE_TBB=ON) and
PaRSEC (-DENABLE_PARSEC=ON), ideally we can abstract out what needs to
happen inside MADNESS itself
On Fri, Jul 9, 2021 at 12:56 PM Andrey Asadchev ***@***.***>
wrote:
> I wanted to hide madness from mpqc proper, st if taskq changes Only TA is
> affected
>
> On Jul 9, 2021 11:40 AM, "Eduard Valeyev" ***@***.***> wrote:
>
> > ***@***.**** requested changes on this pull request.
> > ------------------------------
> >
> > In src/TiledArray/tiledarray.cpp
> > <
> #294 (comment)
>
> > :
> >
> > > @@ -127,3 +128,16 @@ void TiledArray::ta_abort(const std::string &m)
{
> > std::cerr << m << std::endl;
> > ta_abort();
> > }
> > +
> > +
> > +void TiledArray::set_taskq_wait_busy() {
> >
> > these really should be free functions in madness namespace, I think ...
> > fine otherwise
> >
> > —
> > You are receiving this because you authored the thread.
> > Reply to this email directly, view it on GitHub
> > <
> #294 (review)-
703120617
> >,
> > or unsubscribe
> > <
> https://github.com/notifications/unsubscribe-auth/
ABH62KBSWXMZ447KSQKX6VDTW4C6RANCNFSM5AC23DXA
> >
> > .
> >
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <https://github.com/ValeevGroup/tiledarray/pull/
294#issuecomment-877324290>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/
AAQXIZ42EWP4KJUP3XCGPO3TW4S5DANCNFSM5AC23DXA>
> .
>
--
web: valeyev.net
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABH62KGLRB44CJI4ETWZIFTTW4UJHANCNFSM5AC23DXA>
.
|
|
yes, I understand. The only use of ConditionVariables that I see is in
DQueue.
all I am saying that MADNESS needs the same wrappers (madness::
set_taskq_wait_busy(), etc.)
On Fri, Jul 9, 2021 at 1:11 PM Andrey Asadchev ***@***.***>
wrote:
… There are functions in madness as well, static in ConditionVariable.
Reason they there is because they affect all CVs not just taskq
On Jul 9, 2021 2:08 PM, "Eduard Valeyev" ***@***.***> wrote:
> NWX plan to use MADNESS runtime directly so they will need to be able to
do
> this also ... + only MADNESS knows how its taskq is implemented, the
> default is to use threads, but it can also use TBB (-DENABLE_TBB=ON) and
> PaRSEC (-DENABLE_PARSEC=ON), ideally we can abstract out what needs to
> happen inside MADNESS itself
>
> On Fri, Jul 9, 2021 at 12:56 PM Andrey Asadchev ***@***.***>
> wrote:
>
> > I wanted to hide madness from mpqc proper, st if taskq changes Only TA
is
> > affected
> >
> > On Jul 9, 2021 11:40 AM, "Eduard Valeyev" ***@***.***> wrote:
> >
> > > ***@***.**** requested changes on this pull request.
> > > ------------------------------
> > >
> > > In src/TiledArray/tiledarray.cpp
> > > <
> >
#294 (comment)
> >
> > > :
> > >
> > > > @@ -127,3 +128,16 @@ void TiledArray::ta_abort(const std::string
&m)
> {
> > > std::cerr << m << std::endl;
> > > ta_abort();
> > > }
> > > +
> > > +
> > > +void TiledArray::set_taskq_wait_busy() {
> > >
> > > these really should be free functions in madness namespace, I think
...
> > > fine otherwise
> > >
> > > —
> > > You are receiving this because you authored the thread.
> > > Reply to this email directly, view it on GitHub
> > > <
> > #294 (review)-
> 703120617
> > >,
> > > or unsubscribe
> > > <
> > https://github.com/notifications/unsubscribe-auth/
> ABH62KBSWXMZ447KSQKX6VDTW4C6RANCNFSM5AC23DXA
> > >
> > > .
> > >
> >
> > —
> > You are receiving this because your review was requested.
> > Reply to this email directly, view it on GitHub
> > <https://github.com/ValeevGroup/tiledarray/pull/
> 294#issuecomment-877324290>,
> > or unsubscribe
> > <https://github.com/notifications/unsubscribe-auth/
> AAQXIZ42EWP4KJUP3XCGPO3TW4S5DANCNFSM5AC23DXA>
> > .
> >
>
>
> --
> web: valeyev.net
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <
#294 (comment)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/ABH62KGLRB44CJI4ETWZIFTTW4UJHANCNFSM5AC23DXA
>
> .
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQXIZ33ENF4PFW3E5HT57DTW4USLANCNFSM5AC23DXA>
.
--
web: valeyev.net
|
|
Ok can do. Keep TA wrappers?
…On Jul 9, 2021 2:16 PM, "Eduard Valeyev" ***@***.***> wrote:
yes, I understand. The only use of ConditionVariables that I see is in
DQueue.
all I am saying that MADNESS needs the same wrappers (madness::
set_taskq_wait_busy(), etc.)
On Fri, Jul 9, 2021 at 1:11 PM Andrey Asadchev ***@***.***>
wrote:
> There are functions in madness as well, static in ConditionVariable.
> Reason they there is because they affect all CVs not just taskq
>
> On Jul 9, 2021 2:08 PM, "Eduard Valeyev" ***@***.***> wrote:
>
> > NWX plan to use MADNESS runtime directly so they will need to be able
to
> do
> > this also ... + only MADNESS knows how its taskq is implemented, the
> > default is to use threads, but it can also use TBB (-DENABLE_TBB=ON)
and
> > PaRSEC (-DENABLE_PARSEC=ON), ideally we can abstract out what needs to
> > happen inside MADNESS itself
> >
> > On Fri, Jul 9, 2021 at 12:56 PM Andrey Asadchev ***@***.***>
> > wrote:
> >
> > > I wanted to hide madness from mpqc proper, st if taskq changes Only
TA
> is
> > > affected
> > >
> > > On Jul 9, 2021 11:40 AM, "Eduard Valeyev" ***@***.***> wrote:
> > >
> > > > ***@***.**** requested changes on this pull request.
> > > > ------------------------------
> > > >
> > > > In src/TiledArray/tiledarray.cpp
> > > > <
> > >
> #294 (comment)
> > >
> > > > :
> > > >
> > > > > @@ -127,3 +128,16 @@ void TiledArray::ta_abort(const std::string
> &m)
> > {
> > > > std::cerr << m << std::endl;
> > > > ta_abort();
> > > > }
> > > > +
> > > > +
> > > > +void TiledArray::set_taskq_wait_busy() {
> > > >
> > > > these really should be free functions in madness namespace, I think
> ...
> > > > fine otherwise
> > > >
> > > > —
> > > > You are receiving this because you authored the thread.
> > > > Reply to this email directly, view it on GitHub
> > > > <
> > > https://github.com/ValeevGroup/tiledarray/pull/
294#pullrequestreview-
> > 703120617
> > > >,
> > > > or unsubscribe
> > > > <
> > > https://github.com/notifications/unsubscribe-auth/
> > ABH62KBSWXMZ447KSQKX6VDTW4C6RANCNFSM5AC23DXA
> > > >
> > > > .
> > > >
> > >
> > > —
> > > You are receiving this because your review was requested.
> > > Reply to this email directly, view it on GitHub
> > > <https://github.com/ValeevGroup/tiledarray/pull/
> > 294#issuecomment-877324290>,
> > > or unsubscribe
> > > <https://github.com/notifications/unsubscribe-auth/
> > AAQXIZ42EWP4KJUP3XCGPO3TW4S5DANCNFSM5AC23DXA>
> > > .
> > >
> >
> >
> > --
> > web: valeyev.net
> >
> > —
> > You are receiving this because you authored the thread.
> > Reply to this email directly, view it on GitHub
> > <
> https://github.com/ValeevGroup/tiledarray/pull/
294#issuecomment-877331285
> >,
> > or unsubscribe
> > <
> https://github.com/notifications/unsubscribe-auth/
ABH62KGLRB44CJI4ETWZIFTTW4UJHANCNFSM5AC23DXA
> >
> > .
> >
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <https://github.com/ValeevGroup/tiledarray/pull/
294#issuecomment-877332779>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/
AAQXIZ33ENF4PFW3E5HT57DTW4USLANCNFSM5AC23DXA>
> .
>
--
web: valeyev.net
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABH62KFF32EJF3AOWLZHVSTTW4VF5ANCNFSM5AC23DXA>
.
|
|
That's fine, just redirect to madness ones
On Fri, Jul 9, 2021, 1:17 PM Andrey Asadchev ***@***.***>
wrote:
… Ok can do. Keep TA wrappers?
On Jul 9, 2021 2:16 PM, "Eduard Valeyev" ***@***.***> wrote:
> yes, I understand. The only use of ConditionVariables that I see is in
> DQueue.
>
> all I am saying that MADNESS needs the same wrappers (madness::
> set_taskq_wait_busy(), etc.)
>
> On Fri, Jul 9, 2021 at 1:11 PM Andrey Asadchev ***@***.***>
> wrote:
>
> > There are functions in madness as well, static in ConditionVariable.
> > Reason they there is because they affect all CVs not just taskq
> >
> > On Jul 9, 2021 2:08 PM, "Eduard Valeyev" ***@***.***> wrote:
> >
> > > NWX plan to use MADNESS runtime directly so they will need to be able
> to
> > do
> > > this also ... + only MADNESS knows how its taskq is implemented, the
> > > default is to use threads, but it can also use TBB (-DENABLE_TBB=ON)
> and
> > > PaRSEC (-DENABLE_PARSEC=ON), ideally we can abstract out what needs
to
> > > happen inside MADNESS itself
> > >
> > > On Fri, Jul 9, 2021 at 12:56 PM Andrey Asadchev ***@***.***>
> > > wrote:
> > >
> > > > I wanted to hide madness from mpqc proper, st if taskq changes Only
> TA
> > is
> > > > affected
> > > >
> > > > On Jul 9, 2021 11:40 AM, "Eduard Valeyev" ***@***.***> wrote:
> > > >
> > > > > ***@***.**** requested changes on this pull request.
> > > > > ------------------------------
> > > > >
> > > > > In src/TiledArray/tiledarray.cpp
> > > > > <
> > > >
> >
#294 (comment)
> > > >
> > > > > :
> > > > >
> > > > > > @@ -127,3 +128,16 @@ void TiledArray::ta_abort(const
std::string
> > &m)
> > > {
> > > > > std::cerr << m << std::endl;
> > > > > ta_abort();
> > > > > }
> > > > > +
> > > > > +
> > > > > +void TiledArray::set_taskq_wait_busy() {
> > > > >
> > > > > these really should be free functions in madness namespace, I
think
> > ...
> > > > > fine otherwise
> > > > >
> > > > > —
> > > > > You are receiving this because you authored the thread.
> > > > > Reply to this email directly, view it on GitHub
> > > > > <
> > > > https://github.com/ValeevGroup/tiledarray/pull/
> 294#pullrequestreview-
> > > 703120617
> > > > >,
> > > > > or unsubscribe
> > > > > <
> > > > https://github.com/notifications/unsubscribe-auth/
> > > ABH62KBSWXMZ447KSQKX6VDTW4C6RANCNFSM5AC23DXA
> > > > >
> > > > > .
> > > > >
> > > >
> > > > —
> > > > You are receiving this because your review was requested.
> > > > Reply to this email directly, view it on GitHub
> > > > <https://github.com/ValeevGroup/tiledarray/pull/
> > > 294#issuecomment-877324290>,
> > > > or unsubscribe
> > > > <https://github.com/notifications/unsubscribe-auth/
> > > AAQXIZ42EWP4KJUP3XCGPO3TW4S5DANCNFSM5AC23DXA>
> > > > .
> > > >
> > >
> > >
> > > --
> > > web: valeyev.net
> > >
> > > —
> > > You are receiving this because you authored the thread.
> > > Reply to this email directly, view it on GitHub
> > > <
> > https://github.com/ValeevGroup/tiledarray/pull/
> 294#issuecomment-877331285
> > >,
> > > or unsubscribe
> > > <
> > https://github.com/notifications/unsubscribe-auth/
> ABH62KGLRB44CJI4ETWZIFTTW4UJHANCNFSM5AC23DXA
> > >
> > > .
> > >
> >
> > —
> > You are receiving this because your review was requested.
> > Reply to this email directly, view it on GitHub
> > <https://github.com/ValeevGroup/tiledarray/pull/
> 294#issuecomment-877332779>,
> > or unsubscribe
> > <https://github.com/notifications/unsubscribe-auth/
> AAQXIZ33ENF4PFW3E5HT57DTW4USLANCNFSM5AC23DXA>
> > .
> >
>
>
> --
> web: valeyev.net
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <
#294 (comment)
>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/ABH62KFF32EJF3AOWLZHVSTTW4VF5ANCNFSM5AC23DXA
>
> .
>
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#294 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQXIZ2HQM5YVP2ZE7UEQVTTW4VJ7ANCNFSM5AC23DXA>
.
|
asadchev
force-pushed
the
asadchev/improve/taskq-wait-policy
branch
4 times, most recently
from
10b316a to
82359ad
Compare
evaleev
approved these changes
Jul 12, 2021
asadchev
force-pushed
the
asadchev/improve/taskq-wait-policy
branch
from
82359ad to
d52db77
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.