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

Support Caliper based profiling #741

Merged
merged 8 commits into from Aug 3, 2016

Conversation

Projects
None yet
5 participants
@grondo
Copy link
Contributor

grondo commented Aug 2, 2016

This PR is a continuation of #672, which extends the work of @garlick and @trws with

  • fix missing pkg-config argument when building with libcaliper-stub
  • New option --caliper-profile=ARG to flux start, which exports proper LD_PRELOAD, CALI_CONFIG_PROFILE=ARG and CALI_LOG_VERBOSITY=0 to brokers (CALI_* vars are not overwritten)

Also, when I was working another angle I abstracted the caliper init routines for broker and modules into their own functions. I left that in here, but could easily remove it as it isn't necessary right now (thought I could get runtime initialization working)

I also realize I need to document the --caliper-profile option to flux-start.

@grondo grondo added the review label Aug 2, 2016

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Aug 2, 2016

Oh, I also normalized the commit messages with a profiling: topic (we had profiling: tracing: and caliper:, I just picked one)

@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage decreased (-0.02%) to 75.196% when pulling 57f5d4f on grondo:trws-tracing into 088ba24 on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage decreased (-0.02%) to 75.196% when pulling 9bc2bd2 on grondo:trws-tracing into 088ba24 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Aug 2, 2016

This is looking like a good first cut to me. Possibly the commits could be massaged a bit to split the build system and hook additions, and squash incremental development, but if it's a pain or if we want to preserve @trws's line of thought to make it easier for him to review later, I"m OK with leaving it as is - it's just not that much change overall.

Does this still require a non-released version of caliper to build?

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Aug 3, 2016

Possibly the commits could be massaged a bit to split the build system and hook additions, and squash incremental development,

Yeah, I think I could try to do that and still preserve @trws's authorship. If not, I propose leaving as is (though I could squash my typo fix into his)

I will test with other versions of Caliper, but AFAIK, it does require something recent from Caliper git master. I did get that building in Travis a little while ago so we could at least ensure the caliper support builds and works in Travis.

garlick and others added some commits Aug 3, 2016

build: Add --enable-caliper flag to ./configure
Add an --enable-caliper option to ./configure to check for
Caliper[1] support for use in profiling/tracing.

Define caliper libs and flags accordingly.

[1] https://github.com/LLNL/Caliper
build: use pkg-config for libcaliper, link only to stubs
Switch to the use of pkg-config for libcaliper, and link only to
libcaliper-stub by default. Full tracing can be enabled by preloading
the libcaliper full symbols with LD_PRELOAD=libcaliper.so
profiling: initial tracing support via libcaliper
Add initial profiling support using libcaliper, creating a context
for the broker main thread as well as module threads.

Additionally, messages are traced via the dispatch code.
broker: avoid tracing children of broker
Unset CALI_SERVICES_ENABLE and CALI_CONFIG_PROFILE in flux-broker
after profiling initialization to avoid tracing of child processes.
profiling: enhance message tracing
This update adds tracing for various properties of messages, RPCs,
events, etc.  This will not give us all we need to get matches for
all messages, but it's on the way.

As an FYI, be aware that the profiling_msg_snapshot routine can
be called due to messaging activity inside connector_init, so the
profiling object is not ready, and potentially cause uninitialized
nodes to be emitted.  This is currently addressed by a check and return
in the snapshot routine, but if at some point those code-paths become
threaded something fancier may be required.
broker: abstract profiling initialization
Abstract broker and module caliper initialization into their
own functions.
cmd/flux-start: Add --caliper-profile option
Add --caliper-profile=PROFILE option to flux-start, which causes

 LD_PRELOAD=libcaliper.so

and

 CALI_CONFIG_PROFILE=$PROFILE

to be set in the environment before invoking brokers. If Flux was
not built with --enable-caliper, then this option issues an error.

Unless already set in environment, CALI_LOG_VERBOSITY=0 is also set
by default to quiet the Caliper logs to stderr.
doc: document --caliper-profile option for flux-start
In flux-start(1), document the --caliper-profile option

@grondo grondo force-pushed the grondo:trws-tracing branch from 9bc2bd2 to 2dc4fe5 Aug 3, 2016

@coveralls

This comment has been minimized.

Copy link

coveralls commented Aug 3, 2016

Coverage Status

Coverage decreased (-0.04%) to 75.173% when pulling 2dc4fe5 on grondo:trws-tracing into 088ba24 on flux-framework:master.

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Aug 3, 2016

Ok, I reworked this PR by removing some of the intermediate state and splitting the build changes from the actual code. Some commits were left separate to preserve authorship. @garlick, let me know if this one is better.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Aug 3, 2016

This does look good. Shall we merge?

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Aug 3, 2016

Sure -- I'm working on some related testing changes which could be a future PR.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Aug 3, 2016

OK, thanks!

@garlick garlick merged commit 3224fe0 into flux-framework:master Aug 3, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 75.173%
Details

@garlick garlick removed the review label Aug 3, 2016

@grondo grondo deleted the grondo:trws-tracing branch Aug 6, 2016

@trws

This comment has been minimized.

Copy link
Member

trws commented Aug 6, 2016

Thanks guys! Sorry I dropped off, Sunday was the day. =)


Sent from Boxer | http://getboxer.comhttp://bxr.io/PBI3C

On August 3, 2016 at 1:02:31 PM PDT, Jim Garlick notifications@github.com wrote:

Merged #741#741.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/741#event-744370815, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAoStQa50hr3n1cuHWC7hc7c0K0cgECQks5qcPO_gaJpZM4JbH5z.

@springme

This comment has been minimized.

Copy link

springme commented Aug 6, 2016

Congratulations, Tom! Tapasya showed us the picture of Baby Kimberly and she is beautiful. We are all very excited for you and Britany and not the least worried that you dropped off flux threads for a while. Take your time and enjoy this special family time. When you do return please bring pictures!

Becky

Sent with Good (www.good.com)


From: Tom Scogland
Sent: Saturday, August 06, 2016 9:22:06 AM
To: flux-framework/flux-core
Subject: Re: [flux-framework/flux-core] Support Caliper based profiling (#741)

Thanks guys! Sorry I dropped off, Sunday was the day. =)


Sent from Boxer | http://getboxer.comhttp://bxr.io/PBI3C

On August 3, 2016 at 1:02:31 PM PDT, Jim Garlick notifications@github.com wrote:

Merged #741#741.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/741#event-744370815, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAoStQa50hr3n1cuHWC7hc7c0K0cgECQks5qcPO_gaJpZM4JbH5z.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHubhttps://github.com//pull/741#issuecomment-238031623, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEdU0OKkPhQ6ZQ8izTbLm-gU41eZM3Dzks5qdLSugaJpZM4JbH5z.

@trws

This comment has been minimized.

Copy link
Member

trws commented Aug 6, 2016

I certainly will, thanks very much Becky!

-Tom

On August 6, 2016 at 9:42:36 AM PDT, Becky Springmeyer notifications@github.com wrote:
Congratulations, Tom! Tapasya showed us the picture of Baby Kimberly and she is beautiful. We are all very excited for you and Britany and not the least worried that you dropped off flux threads for a while. Take your time and enjoy this special family time. When you do return please bring pictures!

Becky

Sent with Good (www.good.com)


From: Tom Scogland
Sent: Saturday, August 06, 2016 9:22:06 AM
To: flux-framework/flux-core
Subject: Re: [flux-framework/flux-core] Support Caliper based profiling (#741)

Thanks guys! Sorry I dropped off, Sunday was the day. =)


Sent from Boxer | http://getboxer.comhttp://bxr.io/PBI3C

On August 3, 2016 at 1:02:31 PM PDT, Jim Garlick notifications@github.com wrote:

Merged #741#741.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/741#event-744370815, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAoStQa50hr3n1cuHWC7hc7c0K0cgECQks5qcPO_gaJpZM4JbH5z.

You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHubhttps://github.com//pull/741#issuecomment-238031623, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEdU0OKkPhQ6ZQ8izTbLm-gU41eZM3Dzks5qdLSugaJpZM4JbH5z.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/741#issuecomment-238032679, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AAoStblX_JcQsEl0Nx-LgR53yFh8J1p-ks5qdLl0gaJpZM4JbH5z.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.