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

Export gProfiler-needed functionality from asprof #7

Conversation

marcin-ol
Copy link

Experimental merge of upstream changes to async-profiler.
Implement changes necessary to support upcoming features (esp. binary launcher - asprof) in gprofiler.

This is approach 1 - adding jattach passthru in asprof launcher.

@Jongy
Copy link

Jongy commented Apr 25, 2023

@marcin-ol I pushed the rebase to async-profiler-granulate, I'm not sure the rebase is perfect - will validate it later - meanwhile you can re-base your branch on it and add your commits?

@marcin-ol marcin-ol force-pushed the tryout-asprof-jattach-passthru branch from d0cad8b to 6e66b65 Compare April 26, 2023 09:05
@Jongy Jongy force-pushed the async-profiler-granulate branch from 044a094 to 1f31fdc Compare April 27, 2023 08:53
@Jongy
Copy link

Jongy commented Apr 27, 2023

@marcin-ol , I fixed the rebase again so please re-apply the 2 commits of yours. Now it's ready from my end.

This is the full range-diff of the rebase for reference.

$ git range-diff upstream/master..v2.9g6 upstream/master..async-profiler-granulate
 1:  13e835e =  1:  d3a863a Remove the "Profiling started" message
 2:  e16fad0 =  2:  a0a9633 Remove the "Profiling stopped" message
 3:  0c25728 <  -:  ------- Compile statically with libstdc++
 4:  fca73de <  -:  ------- Build jattach statically
 5:  f8f77fd <  -:  ------- Build fdtransfer statically
 6:  ab6e1de <  -:  ------- Compile statically with libgcc
 7:  ae5daf1 <  -:  ------- Fix -static-* args passed only in non-MERGE case :(
 8:  c8c72a1 <  -:  ------- Close standard streams after forking in fdtransfer
 9:  49080a2 <  -:  ------- -static-libstdc++ only on musl
 -:  ------- >  3:  f71df14 Compile statically with libstdc++
 -:  ------- >  4:  bf4337c Compile statically with libgcc
 -:  ------- >  5:  8ebf525 Fix -static-* args passed only in non-MERGE case :(
 -:  ------- >  6:  98280ba Close standard streams after forking in fdtransfer
 -:  ------- >  7:  87f9dff -static-libstdc++ only on musl
10:  f4d874c !  8:  99e87c8 Convert frame name format to 'function (DSO)' for LIB style (#2)
    @@ src/codeCache.h: class FrameDesc;
     
      ## src/frameName.cpp ##
     @@ src/frameName.cpp: const char* FrameName::decodeNativeSymbol(const char* name) {
    -         char* demangled = abi::__cxa_demangle(name, NULL, NULL, &status);
    +         char* demangled = Demangle::demangle(name);
              if (demangled != NULL) {
                  if (lib_name != NULL) {
     -                _str.assign(lib_name).append("`").append(demangled);
11:  bb06e23 <  -:  ------- Add accept timeout as 3rd parameter of fdtransfer
12:  faaabf8 !  9:  f83d0dc Use compat-glibc on x86_64 glibc
    @@ Commit message
         Use compat-glibc on x86_64 glibc
     
      ## Makefile ##
    -@@ Makefile: $(PACKAGE_DIR): build/$(LIB_PROFILER) build/$(JATTACH) $(FDTRANSFER_BIN) \
    - build:
    - 	mkdir -p build
    +@@ Makefile: build/$(LAUNCHER): src/launcher/* src/jattach/* src/fdtransfer.h
    + 	$(CC) $(CPPFLAGS) $(CFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" -DSUPPRESS_OUTPUT -o $@ src/launcher/*.cpp src/jattach/*.c
    + 	strip $@
      
     -PROFILER_STATIC_FLAGS=-static-libgcc
     +PROFILER_FLAGS=-static-libgcc
    @@ Makefile: $(PACKAGE_DIR): build/$(LIB_PROFILER) build/$(JATTACH) $(FDTRANSFER_BI
     +  endif
      endif
      
    - build/$(LIB_PROFILER_SO): $(SOURCES) $(HEADERS) $(RESOURCES) $(JAVA_HELPER_CLASSES)
    + build/$(LIB_PROFILER): $(SOURCES) $(HEADERS) $(RESOURCES) $(JAVA_HELPER_CLASSES)
      ifeq ($(MERGE),true)
      	for f in src/*.cpp; do echo '#include "'$$f'"'; done |\
    --	$(CXX) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_STATIC_FLAGS) -o $@ -xc++ - $(LIBS)
    -+	$(CXX) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_FLAGS) -o $@ -xc++ - $(LIBS)
    +-	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_STATIC_FLAGS) -o $@ -xc++ - $(LIBS)
    ++	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_FLAGS) -o $@ -xc++ - $(LIBS)
      else
    --	$(CXX) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_STATIC_FLAGS) -o $@ $(SOURCES) $(LIBS)
    -+	$(CXX) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_FLAGS) -o $@ $(SOURCES) $(LIBS)
    +-	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_STATIC_FLAGS) -o $@ $(SOURCES) $(LIBS)
    ++	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared $(PROFILER_FLAGS) -o $@ $(SOURCES) $(LIBS)
      endif
      
    - build/$(JATTACH): src/jattach/*.c src/jattach/*.h
    + build/$(API_JAR): $(API_SOURCES)
13:  df5df4a = 10:  6778799 dump stats on stop
14:  60cbc12 <  -:  ------- Do not try to parse mapped pseudofiles
15:  4938ce8 = 11:  1f31fdc Add option to include method modifiers in frame name (#5)

What's gone as part of this rebase is Add accept timeout as 3rd parameter of fdtransfer - let's make sure your upgraded version supports the same behavior?

Other than that change, other parts should be alright and I trust our tests.

- make sure fdtransfer parent process finishes,
- support prefix directory for async-profiler sharedlib
@marcin-ol marcin-ol force-pushed the tryout-asprof-jattach-passthru branch from 6e66b65 to a0205eb Compare April 28, 2023 22:03
@marcin-ol marcin-ol marked this pull request as ready for review April 28, 2023 22:07
Copy link

@Jongy Jongy left a comment

Choose a reason for hiding this comment

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

Done reviewing

Copy link

@Jongy Jongy left a comment

Choose a reason for hiding this comment

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

I did another review round. Left a bunch of small comments. Also, I re-reviewed my rebase. All looks good! Let's finish those and merge it :)

@Jongy Jongy merged commit b3baf60 into Granulate:async-profiler-granulate May 10, 2023
@Jongy Jongy changed the title Tryout asprof jattach passthru Export gProfiler-needed functionality from asprof May 10, 2023
Jongy pushed a commit that referenced this pull request Jan 27, 2024
In AP 3.0, @Jongy edited this commit to export fdtransfer from asprof, as jattach
and jcmd were added in async-profiler@b8a60e6
to upstream.
Jongy pushed a commit that referenced this pull request Jan 27, 2024
In AP 3.0, @Jongy edited this commit to export fdtransfer from asprof, as jattach
and jcmd were added in async-profiler@b8a60e6
to upstream.
Jongy pushed a commit that referenced this pull request Jan 27, 2024
In AP 3.0, @Jongy edited this commit to export fdtransfer from asprof, as jattach
and jcmd were added in async-profiler@b8a60e6
to upstream.
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