ittnotify: use untouched library

[clementperon]

Description

I build Intel oneTBB and Intel ITT in the same project.
Leading to symbols defined multiple times at link time.

Fixes # -
Explicit link with ittapi::ittnotify library
Avoid duplicating the library if ittapi::ittnotify already exist

Type of change

Explicit the ittapi library and properly fetch it.

Add a respective label(s) to PR if you have permissions

• bug fix - change that fixes an issue
• new feature - change that adds functionality
• tests - change in tests
• infrastructure - change in infrastructure and CI
• documentation - documentation update

Tests

• added - required for new features and some bug fixes
• not needed

Documentation

• updated in # - add PR number
• needs to be updated
• not needed

Breaks backward compatibility

• Yes
• No
• Unknown

Notify the following users

@JhaShweta1

Other information

[dnmokhov]

@clementperon, the CI is showing link errors.

[clementperon]

@clementperon, the CI is showing link errors.

Sorry indeed, Let me check on my CI first.
But do you agree with the idea ?

[pavelkumbrasev]

I don't think we can remove ittnotify from TBB

[clementperon]

I don't think we can remove ittnotify from TBB

@pavelkumbrasev My plan is not to remove it but to link with a proper library/target.

In my project i'm linking with both TBB and ITT, and as TBB is redefining ITT symbols. It creates an issue at link time.

[clementperon]

@dnmokhov Sorry for spamming the CI i thought putting it in draft would have stopped the CI to run.
It should now be OK.

in order to clean a bit more I think moving the itt_notify.cpp and itt_notify.h to src/ittapi would be also cleaner.

What do you think?

[clementperon]

@dnmokhov @pavelkumbrasev @isaevil any feedback on this?

[isaevil]

@clementperon I don't really think that storing an archive in the repo is a good idea. It leaves us without the ability to apply particular workarounds if any issues with third-parties occur without waiting for them to be upstreamed (@pavelkumbrasev @dnmokhov what do you think?).

Could you please describe a little bit more your issue with linkage? I've tried locally to fetch both oneTBB and ittnotify in the same CMake project and it worked:

CMakeLists.txt

cmake_minimum_required(VERSION 3.5)
project(sample)

include(FetchContent)

# Fetch oneTBB
FetchContent_Declare(
  oneTBB
  GIT_REPOSITORY https://github.com/oneapi-src/oneTBB.git
  GIT_TAG master
)

FetchContent_Declare(
  ittnotify
  GIT_REPOSITORY https://github.com/intel/ittapi.git
  GIT_TAG master
)

FetchContent_MakeAvailable(oneTBB)
FetchContent_MakeAvailable(ittnotify)

add_executable(sample main.cpp)

target_link_libraries(sample TBB::tbb ittnotify)

main.cpp

#include "tbb/tbb.h"
#include "ittnotify.h"

void pseudoComputation(int n) {
  tbb::parallel_for(0, n, [](int i) {
    for (volatile int j = 0; j < 1000000; ++j)
      ;
  });
}

int main() {
  __itt_resume();
  pseudoComputation(42);
  __itt_pause();
}

[clementperon]

@clementperon I don't really think that storing an archive in the repo is a good idea. It leaves us without the ability to apply particular workarounds if any issues with third-parties occur without waiting for them to be upstreamed (@pavelkumbrasev @dnmokhov what do you think?).

You can still patch an archive with FetchContent()

find_package(Patch REQUIRED)
set(TBB_PATCH_COMMAND_1 ${Patch_EXECUTABLE} -p1 -i
                        "${CMAKE_CURRENT_SOURCE_DIR}/oneTBB_0001.patch")
set(TBB_PATCH_COMMAND_2 ${Patch_EXECUTABLE} -p1 -i
                        "${CMAKE_CURRENT_SOURCE_DIR}/oneTBB_0002.patch")

FetchContent_Declare(
        onetbb
        .....
        PATCH_COMMAND ${TBB_PATCH_COMMAND_1} && ${TBB_PATCH_COMMAND_2}
         )

The error i'm trying to deal with is:

/usr/bin/ld: gnu_13.2_cxx20_64_relwithdebinfo/libtbb.a(itt_notify.cpp.o):/my_project/dev/_deps/onetbb-src/src/tbb/tools_api/ittnotify_static.c:290: multiple definition of `__itt__ittapi_global'; bin/libittnotify.a(ittnotify_static.c.o):/my_project/dev/_deps/ittapi-src/src/ittnotify/ittnotify_static.c:280: first defined here
/usr/bin/ld: gnu_13.2_cxx20_64_relwithdebinfo/libtbb.a(itt_notify.cpp.o):/my_project/dev/_deps/onetbb-src/src/tbb/tools_api/ittnotify_static.h:110: multiple definition of `__itt_detach_ptr__3_0'; bin/libittnotify.a(ittnotify_static.c.o):/my_project/dev/_deps/ittapi-src/src/ittnotify/ittnotify_static.h:100: first defined here
/usr/bin/ld: gnu_13.2_cxx20_64_relwithdebinfo/libtbb.a(itt_notify.cpp.o):/my_project/dev/_deps/onetbb-src/src/tbb/tools_api/ittnotify_static.h:118: multiple definition of `__itt_sync_create_ptr__3_0'; bin/libittnotify.a(ittnotify_static.c.o):/my_project/dev/_deps/ittapi-src/src/ittnotify/ittnotify_static.h:108: first defined here
....

I will try to find a small example to reproduce it.