Skip to content

Conversation

@Jongy
Copy link
Contributor

@Jongy Jongy commented Sep 8, 2021

Description

Bases on async-profiler/async-profiler#411 to use AP's CPU mode.

Motivation and Context

We'll have kernel stacks for profiled Java processes :)

How Has This Been Tested?

Locally. I'll add some test as well. added a test

Checklist:

  • I have read the CONTRIBUTING document.
  • Wait for the next AP release (haven't updated it yet)
  • I have updated the relevant documentation.
  • I have added tests for new logic.

@Jongy Jongy added the enhancement New feature or request label Sep 8, 2021
@Jongy Jongy force-pushed the java-async-profiler-fdtransfer branch from 7ed8c80 to d7d12bd Compare October 4, 2021 07:53
@Jongy Jongy marked this pull request as ready for review October 4, 2021 07:53
@Jongy Jongy requested a review from michelhe October 4, 2021 07:53
@Jongy
Copy link
Contributor Author

Jongy commented Oct 4, 2021

Default to itimer so we can use the CPU mode manually in some deployments, until we gain confidence in it.

# just wait for the process to exit
process.wait()
else:
assert communicate
Copy link

Choose a reason for hiding this comment

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

assert with message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@michelhe
Copy link

michelhe commented Oct 4, 2021

AP build seems to fail in CI

Copy link

@michelhe michelhe left a comment

Choose a reason for hiding this comment

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

.

@Jongy
Copy link
Contributor Author

Jongy commented Oct 4, 2021

$ git range-diff v2.5..v2.5g1 v2.0..v2.0g5
1:  5f90814 ! 1:  ab1b8c2 Replace semicolon in method descriptors to vertical bar, since a semicolon is already used to separate between methods
    @@ src/profiler.cpp: void Profiler::dumpCollapsed(std::ostream& out, Arguments& arg
     +            std::replace(frame_name.begin(), frame_name.end(), ';', '|');
                  out << frame_name << (j == 0 ? ' ' : ';');
              }
    -         out << samples << "\n";
    +         out << (args._counter == COUNTER_SAMPLES ? (*it)->samples : (*it)->counter) << "\n";
2:  b3ff118 ! 2:  9692c57 Remove the "Profiling started" message
    @@ src/profiler.cpp: Error Profiler::runInternal(Arguments& args, std::ostream& out
                  if (error) {
                      return error;
                  }
    --            out << "Profiling started\n";
    +-            out << "Profiling started" << std::endl;
                  break;
              }
              case ACTION_STOP: {
3:  d10c12a < -:  ------- Remove the "Profiling stopped" message
-:  ------- > 3:  51447a8 Remove the "Profiling stopped" message
4:  d7f1029 ! 4:  6483566 Compile statically with libstdc++
    @@ Makefile: build:
     -	$(CXX) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared -o $@ $(SOURCES) $(LIBS)
     +	$(CXX) $(CXXFLAGS) -DPROFILER_VERSION=\"$(PROFILER_VERSION)\" $(INCLUDES) -fPIC -shared -static-libstdc++ -o $@ $(SOURCES) $(LIBS)
      
    - build/$(JATTACH): src/jattach/*.c src/jattach/*.h
    - 	$(CC) $(CFLAGS) -DJATTACH_VERSION=\"$(PROFILER_VERSION)-ap\" -o $@ src/jattach/*.c
    + build/$(JATTACH): src/jattach/jattach.c
    + 	$(CC) $(CFLAGS) -DJATTACH_VERSION=\"$(PROFILER_VERSION)-ap\" -o $@ $^
5:  b9645a7 < -:  ------- Build jattach statically
-:  ------- > 5:  9d29169 Build jattach statically
6:  e70054f ! 6:  3f1505b Enocde buildid+offset of unknown symbols in the frame (#1)
    @@ src/arguments.h: class Arguments {
              _buf(NULL),
     @@ src/arguments.h: class Arguments {
              _end(NULL),
    -         _title(NULL),
    +         _title("Flame Graph"),
              _minwidth(0),
     -        _reverse(false) {
     +        _reverse(false),
    @@ src/codeCache.cpp
      #include "codeCache.h"
      
      
    -@@ src/codeCache.cpp: NativeCodeCache::NativeCodeCache(const char* name, short lib_index, const void*
    -     _lib_index = lib_index;
    +@@ src/codeCache.cpp: NativeCodeCache::NativeCodeCache(const char* name, const void* min_address, cons
    +     _name = strdup(name);
          _min_address = min_address;
          _max_address = max_address;
     +    _build_id = NULL;
    @@ src/codeCache.cpp: NativeCodeCache::NativeCodeCache(const char* name, short lib_
      NativeCodeCache::~NativeCodeCache() {
     +    free(_build_id);
          for (int i = 0; i < _count; i++) {
    -         free((char*)_blobs[i]._method - LIB_INDEX_OFFSET);
    +         free(_blobs[i]._method);
          }
     @@ src/codeCache.cpp: void NativeCodeCache::sort() {
          if (_max_address == NO_MAX_ADDRESS) _max_address = _blobs[_count - 1]._end;
    @@ src/codeCache.h
      
      #define NO_MIN_ADDRESS  ((const void*)-1)
      #define NO_MAX_ADDRESS  ((const void*)0)
    -@@ src/codeCache.h: class NativeCodeCache : public CodeCache {
    +@@ src/codeCache.h: class CodeCache {
    + class NativeCodeCache : public CodeCache {
        private:
          char* _name;
    -     short _lib_index;
     +    char *_build_id;
      
    -     static char* encodeLibrarySymbol(const char* name, short lib_index);
    - 
    +   public:
    +     NativeCodeCache(const char* name,
     @@ src/codeCache.h: class NativeCodeCache : public CodeCache {
      
          void add(const void* start, int length, const char* name, bool update_bounds = false);
    @@ src/profiler.cpp: Error Profiler::start(Arguments& args, bool reset) {
     
      ## src/profiler.h ##
     @@ src/profiler.h: class Profiler {
    +     CStack _cstack;
          bool _add_thread_frame;
    -     bool _add_sched_frame;
          bool _update_thread_names;
     +    bool _add_build_ids;
          volatile bool _thread_events_state;
7:  02cb785 < -:  ------- Build fdtransfer statically

@Jongy
Copy link
Contributor Author

Jongy commented Oct 4, 2021

AP build seems to fail in CI

Hehe didn't push it

@Jongy
Copy link
Contributor Author

Jongy commented Oct 4, 2021

Re-ran, now it'll build successfully.

@Jongy Jongy requested a review from michelhe October 4, 2021 11:08
michelhe
michelhe previously approved these changes Oct 4, 2021
Comment on lines 30 to 31
_mmap_size_fp = 129 # default number of pages used by "perf record" when perf_event_mlock_kb=516
_mmap_size_dwarf = 257 # doubling the previous
Copy link

Choose a reason for hiding this comment

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

cleaner

_mmap_sizes = { 'fp': 129, 'dwarf': 257 }

....

str(self._mmap_sizes[self._type])

@Jongy Jongy requested a review from michelhe October 6, 2021 15:57
@Jongy Jongy changed the title java: async-profiler CPU mode with fdtransfer java: async-profiler v2.5 and CPU mode with fdtransfer Oct 7, 2021
@Jongy
Copy link
Contributor Author

Jongy commented Oct 7, 2021

Maybe now will pass? lol

Copy link

@michelhe michelhe left a comment

Choose a reason for hiding this comment

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

kool

@Jongy Jongy merged commit 1f85372 into master Oct 10, 2021
@Jongy Jongy deleted the java-async-profiler-fdtransfer branch October 10, 2021 08:08
Jongy added a commit that referenced this pull request Mar 27, 2023
This is unsafe. It's not used now anyway - was added in #175 for fdtransfer, but since its only usage was removed
Jongy added a commit that referenced this pull request Mar 29, 2023
* utils: Add kwargs-only to run_process
* utils: run_process(): Remove communicate arg - this is unsafe. It's not used now anyway - was added in #175 for fdtransfer, but since then its only usage was removed.
* utils: reap_process(): Avoid wait()
* utils: Extract reap_process()
* php: use reap_process()
* perf: use reap_process()
* python_ebpf: Use reap_process()

This possibly fixes #744.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request runtime/java

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants