add C API for profiling

[eddy16112]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 87.37705% with 77 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.61%. Comparing base (b9df3d6) to head (9586804).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
tests/unit_tests/c_profiling.cc	91.56%	28 Missing ⚠️
src/realm/realm_c.cc	76.57%	22 Missing and 4 partials ⚠️
src/realm/profiling.inl	87.60%	13 Missing and 2 partials ⚠️
src/realm/profiling.cc	30.00%	7 Missing ⚠️
tests/unit_tests/c_inst.cc	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #269      +/-   ##
==========================================
+ Coverage   27.15%   28.61%   +1.45%     
==========================================
  Files         190      191       +1     
  Lines       39174    39759     +585     
  Branches    14166    14308     +142     
==========================================
+ Hits        10638    11377     +739     
- Misses      27240    27350     +110     
+ Partials     1296     1032     -264     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[muraj]

Can you move the comment so this isn't split into two lines?

And these type names are getting insanely long. Do we really need to call it "realm_profiling_measurement_x_y_z"? Can't we just call these "realm_prof_x_y_z" ?

[muraj]

And you can combine copy_info_inst_info into just _copy_inst_info.

[eddy16112]

Yeah, the name is too long. I renamed them to realm_profiling_x_y_z, just because personally I would prefer the full English words as it is easy to understand.

[muraj]

Documentation makes it easier to understand. Most codes will shorten the type names significantly. We don't need full english names. Like in this case, the following is enough:

realm_prof_copy_info_t

There's no need to add request_type or measurement_operation, as it is just redundant and doesn't add any value that documentation wouldn't give.

[eddy16112]

The measurement isn’t very useful, so I removed it. However, it’s better to keep operation (or op)—I prefer operation because we generally use full names elsewhere, e.g., instance, processor and etc, and this keeps the naming consistent. Having operation makes it immediately clear that the profiling data comes from an operation. Similarly, profiling entries that start with instance indicate that the data comes from instance creation or redistribution. Honestly, I think it’s easier to understand how to use the profiling measurements this way, rather than constantly referring back to the documentation to remember what each class represents. That’s why the C++ naming also starts with Operation or Instance.

Regarding realm_profiling_operation_copy_info_request_type_t, I’m not sure how to shorten the name while still clearly indicating that it relates to copy_info.

[muraj]

Why do you have a structure for this? Do you really need one?

[eddy16112]

because in c++, it is std::vector. Here, the caller will pass a realm_profiling_measurement_operation_backtrace_symbol_t*, otherwise, they will have to pass an array of char symbol[PMID_OP_BACKTRACE_SYMBOLS_MAX_LENGTH];

[muraj]

Yes, they would, and that's okay.

[eddy16112]

But how should the caller know they need to use char symbol[PMID_OP_BACKTRACE_SYMBOLS_MAX_LENGTH];. I think there are many advantages when warp it into a struct.

1. Type safety. if we define a realm_profiling_measurement_operation_backtrace_symbol_t symbols[N];, then symbols[i] is clearly a symbol struct, not just a char*. Otherwise, the users will have to create an array of char[N * PMID_OP_BACKTRACE_SYMBOLS_MAX_LENGTH], and they have to carefully jump PMID_OP_BACKTRACE_SYMBOLS_MAX_LENGTH to access each symbol.
2. Encapsulation and readability. By wrapping the array in a struct, you can give it a meaningful name (symbol), making the code easier to read and maintain. Using an array of array is really hard to read.
3. Compatibility with STL containers. Structs can be stored in std::vector, std::array, or other containers easily, e.g. std::vector< realm_profiling_measurement_operation_backtrace_symbol_t > symbols;

I do not understand why you want to remove this struct.