Rename from nvprof focused connector to nvtx focused connector #175

vlkale · 2023-04-14T01:58:28Z

Fix naming of files and name of tool.

dalg24

Your PR description claims the existing tool is being renamed, but the patch is a pure addition alongside the old facility. Also the build system probably needs some update to accommodate that renaming.

vlkale · 2023-04-14T03:24:58Z

@dalg24 Thanks.This is a draft PR. I ought to have marked it as so. Your comments are on my todo already.

vlkale · 2023-04-14T19:23:45Z

profiling/all/kp_all.cpp

-  handlers["nvprof-connector"] = NVProfConnector::get_event_set();
-  handlers["nvprof-focused-connector"] =
-      NVProfFocusedConnector::get_event_set();
+  handlers["nvprof-connector"]       = NVProfConnector::get_event_set();


This is being fixed in #139

vlkale · 2023-07-21T02:18:18Z

I made the fixes to this PR to get rid of the duplicated files, e.g., nvprof_focused_connector.cpp that are mistakenly sitting alongside the nvtx_* named files. I have also updated the CMakeLists.txt file to read the environment variable KokkosTools_ENABLE_NVTX, which I think would be needed for this PR's renaming, though (as mentioned in one of my comments), other related renaming changes for the CMakeLists.txt - specifically those involving nvprof_connector - are the action items of PR #139.

dalg24 · 2023-07-21T03:05:17Z

> git rev-parse HEAD
fed7b89985be621b7e07cc27646842fb57a5eb0f

> tree profiling
profiling
[...]
├── nvprof-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvprof_connector.cpp
│   └── kp_nvprof_connector_domain.h
├── nvprof-focused-connector                  < This PR claims it is renaming files
│   ├── CMakeLists.txt.                       < but as you can see all of these are still
│   ├── Makefile                              < there.  Please fix it.
│   ├── kp_nvprof_focused_connector.cpp       <
│   └── kp_nvprof_focused_connector_domain.h  <
├── nvtx-focused-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvtx_focused_connector.cpp
│   └── kp_nvtx_focused_connector_domain.h
[...]
23 directories, 88 files

profiling/nvtx-focused-connector/kp_nvtx_focused_connector_domain.h

vlkale · 2023-07-21T04:12:17Z

> git rev-parse HEAD
fed7b89985be621b7e07cc27646842fb57a5eb0f

> tree profiling
profiling
[...]
├── nvprof-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvprof_connector.cpp
│   └── kp_nvprof_connector_domain.h
├── nvprof-focused-connector                  < This PR claims it is renaming files
│   ├── CMakeLists.txt.                       < but as you can see all of these are still
│   ├── Makefile                              < there.  Please fix it.
│   ├── kp_nvprof_focused_connector.cpp       <
│   └── kp_nvprof_focused_connector_domain.h  <
├── nvtx-focused-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvtx_focused_connector.cpp
│   └── kp_nvtx_focused_connector_domain.h
[...]
23 directories, 88 files

@dalg24 OK, got it and thanks for that.

This ought to be fixed now with latest changes pushed to the repo - as seen in the below print. However, now there seems to be a merge conflict for profiling/nvprof-focused-connector/kp_nvprof_focused_connector.cpp and
profiling/nvprof-focused-connector/kp_nvprof_focused_connector_domain.h.

user@system:kokkos-tools-develop vlkale$ git rev-parse HEAD
fed7b89985be621b7e07cc27646842fb57a5eb0f
user@system :kokkos-tools-develop vlkale$ tree profiling/
profiling/
├── all
│   ├── CMakeLists.txt
│   ├── impl
│   │   ├── Kokkos_Profiling_C_Interface.h
│   │   ├── Kokkos_Profiling_DeviceInfo.hpp
│   │   └── Kokkos_Profiling_Interface.hpp
│   ├── kp_all.cpp
│   ├── kp_all.hpp
│   └── kp_core.hpp
[...]
├── nvprof-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvprof_connector.cpp
│   └── kp_nvprof_connector_domain.h
├── nvtx-focused-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvtx_focused_connector.cpp
│   └── kp_nvtx_focused_connector_domain.h
[...]
26 directories, 83 files
s1017105ca:kokkos-tools-develop vlkale$

mistakenly made this change in this PR but it is part of PR kokkos#194

vlkale · 2023-08-05T21:41:07Z

FYI: I have resolved the merge conflicts a few days ago and this is building properly now against the CI tests. Please let me know if there is anything else. I think this is ready unless there are any other comments here. See the result of tree below.

@dalg24

user@system kto-dev-vk % git rev-parse HEAD                                         
50c1b1b0481c75b829f27631a21375c6bbf7eeac
user@system kto-dev-vk % tree profiling                                             
profiling
├── all
│   ├── CMakeLists.txt
│   ├── impl
│   │   ├── Kokkos_Profiling_C_Interface.h
│   │   ├── Kokkos_Profiling_DeviceInfo.hpp
│   │   └── Kokkos_Profiling_Interface.hpp
│   ├── kp_all.cpp
│   ├── kp_all.hpp
│   └── kp_core.hpp
├── caliper-connector
│   └── README.md
├── chrome-tracing
│   ├── CMakeLists.txt
│   ├── Makefile
│   └── kp_chrome_tracing.cpp
├── memory-events
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_memory_events.cpp
│   ├── kp_memory_events.hpp
│   └── kp_timer.hpp
├── memory-hwm
│   ├── CMakeLists.txt
│   ├── Makefile
│   └── kp_hwm.cpp
├── memory-hwm-mpi
│   ├── CMakeLists.txt
│   ├── Makefile
│   └── kp_hwm_mpi.cpp
├── memory-usage
│   ├── CMakeLists.txt
│   ├── Makefile
│   └── kp_memory_usage.cpp
├── nvprof-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvprof_connector.cpp
│   └── kp_nvprof_connector_domain.h
├── nvtx-focused-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_nvtx_focused_connector.cpp
│   └── kp_nvtx_focused_connector_domain.h
├── papi-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── README.md
│   ├── kp_papi_connector.cpp
│   └── kp_papi_connector_domain.h
├── perfetto-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── README.md
│   ├── kokkos.cfg
│   ├── libperfetto-connector.cpp
│   └── perfetto
│       ├── LICENSE
│       ├── perfetto.cc
│       └── perfetto.h
├── roctx-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   └── kp_roctx_connector.cpp
├── simple-kernel-timer
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_json_writer.cpp
│   ├── kp_kernel_info.h
│   ├── kp_kernel_timer.cpp
│   ├── kp_kernel_timer_json.cpp
│   ├── kp_reader.cpp
│   ├── kp_shared.cpp
│   └── kp_shared.h
├── space-time-stack
│   ├── CMakeLists.txt
│   ├── Makefile
│   └── kp_space_time_stack.cpp
├── systemtap-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kokkos_stap_deep_copy.stp
│   ├── kokkos_stap_example1.stp
│   ├── kokkos_stap_mon.stp
│   ├── kp_systemtap_connector.cpp
│   └── probes.d
├── timemory-connector
│   └── README.md
├── variorum-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── README.md
│   └── variorum-connector.cpp
├── vtune-connector
│   ├── CMakeLists.txt
│   ├── Makefile
│   ├── kp_vtune_connector.cpp
│   └── kp_vtune_connector_domain.h
└── vtune-focused-connector
    ├── CMakeLists.txt
    ├── Makefile
    ├── kp_vtune_focused_connector.cpp
    └── kp_vtune_focused_connector_domain.h

22 directories, 81 files

Uncomment -lnvToolsExt

Changing add_library to kp_add_library for kernel_filter

Fix kp_kernel_filter to allow KOKKOS_TOOLS_LIBS environment variable

vlkale requested a review from crtrott April 14, 2023 02:05

dalg24 requested changes Apr 14, 2023

View reviewed changes

vlkale marked this pull request as draft April 14, 2023 13:34

vlkale self-assigned this Apr 14, 2023

vlkale added the Naming Fix naming of files, variable names, functions and other elements of code in repo label Apr 14, 2023

vlkale linked an issue Apr 14, 2023 that may be closed by this pull request

Make Kokkos Tools core vendor connectors consistent with all features #174

Open

vlkale removed a link to an issue Apr 14, 2023

Make Kokkos Tools core vendor connectors consistent with all features #174

Open

vlkale mentioned this pull request Apr 14, 2023

Appropriate Fence Tool callbacks in NVprof focused connector #177

Closed

vlkale marked this pull request as ready for review April 14, 2023 19:22

vlkale commented Apr 14, 2023

View reviewed changes

dalg24 reviewed Jul 21, 2023

View reviewed changes

profiling/nvtx-focused-connector/kp_nvtx_focused_connector_domain.h Outdated Show resolved Hide resolved

vlkale closed this Jul 31, 2023

vlkale force-pushed the RenamingNVTXFocusedConnector branch 2 times, most recently from 209d399 to 9b93b9e Compare July 31, 2023 01:21

vlkale added 3 commits July 30, 2023 18:35

putting in all fixes

e590f04

putting changes for nvtx

4c2e200

pushing all nvtx focused connector

909f481

vlkale reopened this Jul 31, 2023

vlkale added 8 commits July 30, 2023 20:14

formatted kp_all

603b6e4

fixing kp all

b203e59

Using kval hashmap (draft) for nestedID

d2a63a1

take out erroneous changes - uniqID

3becbe2

Rollback erroneous change to kp sampler

5066ced

mistakenly made this change in this PR but it is part of PR kokkos#194

formatting to nvtx focused connector

c4f64b5

formatting develop version of sampler that was pasted from develop

3c0a8a9

removing nestedID added by mistake

50c1b1b

vlkale and others added 13 commits August 16, 2023 17:40

fix Makefile to have nvtx

a2258af

updates to kp nvtx focused connector

8c72c1a

change NVPROF to NVTX

9bb7747

applying clang format

ff83acb

adding Makefile

916b8bd

Update Makefile

bab216d

Uncomment -lnvToolsExt

add_library --> kp_add_library

62f2918

Changing add_library to kp_add_library for kernel_filter

Allow deprecated kp_kernel_filter

14d391b

Fix kp_kernel_filter to allow KOKKOS_TOOLS_LIBS environment variable

gix kp kernel with Kokkos Tools libs

f63e8cc

applying clang format

c4f924e

don't include -lnvToolsExt and -lcudart

086f2eb

change tools_globfence variable from int to bool

544522f

applying clang foormat

29f2f23

crtrott closed this Aug 24, 2023

vlkale deleted the RenamingNVTXFocusedConnector branch August 25, 2023 22:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename from nvprof focused connector to nvtx focused connector #175

Rename from nvprof focused connector to nvtx focused connector #175

vlkale commented Apr 14, 2023 •

edited

Loading

dalg24 left a comment

vlkale commented Apr 14, 2023 via email •

edited

Loading

vlkale Apr 14, 2023

vlkale commented Jul 21, 2023 •

edited

Loading

dalg24 commented Jul 21, 2023

vlkale commented Jul 21, 2023 •

edited

Loading

vlkale commented Aug 5, 2023 •

edited

Loading

Rename from nvprof focused connector to nvtx focused connector #175

Rename from nvprof focused connector to nvtx focused connector #175

Conversation

vlkale commented Apr 14, 2023 • edited Loading

dalg24 left a comment

Choose a reason for hiding this comment

vlkale commented Apr 14, 2023 via email • edited Loading

vlkale Apr 14, 2023

Choose a reason for hiding this comment

vlkale commented Jul 21, 2023 • edited Loading

dalg24 commented Jul 21, 2023

vlkale commented Jul 21, 2023 • edited Loading

vlkale commented Aug 5, 2023 • edited Loading

vlkale commented Apr 14, 2023 •

edited

Loading

vlkale commented Apr 14, 2023 via email •

edited

Loading

vlkale commented Jul 21, 2023 •

edited

Loading

vlkale commented Jul 21, 2023 •

edited

Loading

vlkale commented Aug 5, 2023 •

edited

Loading