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

GEOPM service for Caliper #213

Open
wants to merge 6 commits into
base: releases/v2.1.1
Choose a base branch
from

Conversation

amarathe84
Copy link

This PR is for the GEOPM service that we've developed for Caliper. The GEOPM service for Caliper enables transparently linking the GEOPM runtime system (https://github.com/geopm/geopm) for power management with the application through the Caliper markup API. The service captures region attribute updates and maps them to GEOPM application markups. The service handles updates to both OMP and non-OMP regions. The GEOPM markups are listed on this page: https://geopm.github.io/man/geopm_prof_c.3.html. The mapping between Caliper markups and GEOPM markups is listed in src/services/geopm/doc/GEOPM-mapping.pdf.

@codecov-io
Copy link

Codecov Report

Merging #213 into releases/v2.1.1 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           releases/v2.1.1     #213   +/-   ##
================================================
  Coverage            75.67%   75.67%           
================================================
  Files                  123      123           
  Lines                 8065     8065           
================================================
  Hits                  6103     6103           
  Misses                1962     1962

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0593b0e...635b5dc. Read the comment docs.

@@ -42,6 +42,7 @@ option(WITH_GOTCHA "Enable GOTCHA wrapping" TRUE)
option(WITH_SOS "Enable SOSFlow data management" FALSE)
option(WITH_TAU "Enable TAU service (TAU Performance System)" FALSE)
option(WITH_VTUNE "Enable Intel(R) VTune(tm) annotation bindings" FALSE)
option(WITH_GEOPM "Enable GEOPM service" TRUE)
Copy link
Author

Choose a reason for hiding this comment

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

Set TRUE to FALSE

include(FindGeopm)
if(GEOPM_FOUND)
message(STATUS "Found geopm.h in " ${GEOPM_INCLUDE_DIR})
message(STATUS "Found libgeopm.so in " ${GEOPM_LIBRARY})
Copy link
Author

Choose a reason for hiding this comment

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

Replace with GEOPM (check the list of messages at the end of CMakeLists.txt

Copy link
Author

Choose a reason for hiding this comment

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

Check line number CMakeLists.txt: 393

# GEOPM_LIBRARY - Name of GEOPM library file
# GEOPM_FOUND - True if GEOPM was found

#set(GEOPM_INSTALL "" CACHE PATH "GEOPM install directory")
Copy link
Author

Choose a reason for hiding this comment

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

Remove

)

list(APPEND CALIPER_EXTERNAL_LIBS " -DDEBUGLEVEL=2")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DDEBUGLEVEL=2")
Copy link
Author

Choose a reason for hiding this comment

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

Remove (do not usually change CFLAGS

Geopm.cpp
)

list(APPEND CALIPER_EXTERNAL_LIBS " -DDEBUGLEVEL=2")
Copy link
Author

Choose a reason for hiding this comment

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

Remove this


list(APPEND CALIPER_EXTERNAL_LIBS " -DDEBUGLEVEL=2")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DDEBUGLEVEL=2")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DDEBUGLEVEL=2")
Copy link
Author

Choose a reason for hiding this comment

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

Remove this.

Comment on lines +196 to +197
chn->events().pre_begin_evt.connect(::geopm_begin_region);
chn->events().pre_end_evt.connect(::geopm_end_region);
Copy link
Author

Choose a reason for hiding this comment

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

Migrate to annotation binding class (example NVPROF service code).

}
}

if(attr.name() == "annotation") {
Copy link
Author

Choose a reason for hiding this comment

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

Note: need to separate outermost annotation from individual kernel annotations.

/* Start of a loop in the application, initialize loop count
* to update fractional progress later
*/
if(regionname.to_string() == "mainloop") {
Copy link
Author

Choose a reason for hiding this comment

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

Make the outer loop name configuration.

Copy link
Author

@amarathe84 amarathe84 left a comment

Choose a reason for hiding this comment

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

  • Measure overhead of Caliper service + GEOPM.

*/

/* check if the attribute type is annotation, loop or function, and create a region ID. */
if(attr.name() == "annotation") {
Copy link
Author

Choose a reason for hiding this comment

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

Look up built-in attributes by ID instead of strings.


void geopm_begin_region(Caliper* c, Channel* chn, const Attribute& attr, const Variant& regionname) {
int err;
std::string sRegionName(regionname.to_string());
Copy link
Author

Choose a reason for hiding this comment

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

Find out the scope of annotation attribute (process or thread).

if(geopm_phase_map.end() == geopm_phase_map.find(sRegionName)) {
uint64_t phase_rid = GEOPM_NULL_VAL;
geopm_prof_region(sRegionName.c_str(), GEOPM_REGION_HINT_COMPUTE, &phase_rid);
geopm_phase_map.insert(std::pair<std::string, uint64_t>(sRegionName, phase_rid));
Copy link
Author

Choose a reason for hiding this comment

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

Figure out if we can avoid string copy.

* to update fractional progress later
*/
if(regionname.to_string() == "mainloop") {
/* Nothing to do at the start of the mainloop */
Copy link
Author

Choose a reason for hiding this comment

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

Refactor this part of the code

} else {
/* This is neither the main loop nor an OpenMP loop,
* therefore, mark process-level progress */
geopm_prof_progress(geopm_phase_map[sRegionName],
Copy link
Author

Choose a reason for hiding this comment

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

Make sure there is no division by zero (exists check)

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.

2 participants