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

Enhance runtime stats tracking #4144

Merged
merged 9 commits into from
Aug 3, 2022
Merged

Conversation

dipinhora
Copy link
Contributor

@dipinhora dipinhora commented Jun 17, 2022

This PR enhances the runtime stats tracking that was previously
implemented under the USE_MEMTRACK and USED_MEMTRACK_MESSAGES
defines. The new defines are called USE_RUNTIMESTATS and
USE_RUNTIMESTATS_MESSAGES.

Runtime stats tracking tracks the following actor info:

  • heap memory allocated
  • heap memory used
  • heap num allocated
  • heap realloc counter
  • heap alloc counter
  • heap free counter
  • heap gc counter
  • system message processing cpu usage
  • app message processing cpu usage
  • garbage collection cpu usage
  • messages sent counter
  • system messages processed counter
  • app messages processed counter

Runtime tracking tracks the following scheduler info:

  • mutemap memory used
  • mutemap memory allocated
  • memory used for gc acquire/release actormaps and actors created
  • memory allocated for gc acquire/release actormaps and actors created
  • created actors counter
  • destroyed actors counter
  • actor system message processing cpu for all actor runs on the scheduler
  • actor app message processing cpu for all actor runs on the scheduler
  • actor garbage collection cpu for all actor runs on the scheduler
  • scheduler message processing cpu usage
  • scheduler misc cpu usage while waiting to do work
  • memory used by inflight messages
  • memory allocated by inflight messages
  • number of inflight messages

This runtime stats tracking info has been exposed to pony programs as
part of the runtime_info package and an example runtime_info program
has been added to the examples directory.

The runtime stats tracking in a pony program can be used for some
useful validations for those folks concerned about
heap allocations in the critical path (i.e. if they
rely on the compiler's HeapToStack optimization pass
to convert heap allocations to stack allocations and
want to validate it is working correctly).

Example of possible use to validate number of heap allocations:

use "collections"
use "runtime_info"

actor Main
  new create(env: Env) =>
    let num_allocs_before = ActorStats.heap_alloc_counter(ActorStatsAuth(env.root))

    let ret = critical()

    let num_allocs_after = ActorStats.heap_alloc_counter(ActorStatsAuth(env.root))

    env.out.print("Allocations before: " + num_allocs_before.string())
    env.out.print("Allocations after: " + num_allocs_after.string())

    env.out.print("Critical section allocated " + (num_allocs_after - num_allocs_before).string() + " heap objects")

    env.out.print("Allocations at end: " + ActorStats.heap_alloc_counter(ActorStatsAuth(env.root)).string())

  fun critical(): U32 =>
    var x: U32 = 1
    let y: U32 = 1000

    for i in Range[U32](1, y) do
      x = x * y
    end

    x

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jun 17, 2022
@SeanTAllen SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Jun 17, 2022
@SeanTAllen
Copy link
Member

SeanTAllen commented Jun 17, 2022

I'd like to see the new ponyint_ functions exposed via the runtime_info package.

And changed from being ponyint_ to being PONY_API functions.

In the runtime_info, there should be an auth for memory tracking that is required.

@dipinhora
Copy link
Contributor Author

When you say new ponyint_ functions are you referring to only the new ones in this PR or to all of the MEMTRACK related functions that were added when the original PR was merged?

Also, do you like the memtrack name? i'm starting to think that maybe memstats might be better.. what do you think?

@SeanTAllen
Copy link
Member

I was referring to the ones you added but really, any that we expose via the runtime info primitive.

I think memstats is better. For the primitive, perhaps AllocatorInfo ?

@dipinhora
Copy link
Contributor Author

dipinhora commented Jun 18, 2022

Also, if we're going to expose this stuff via PONY_API and runtime_info, should we change the compile settings so that the support for this functionality is compiled/enabled by default (probably not the message level stats tracking due to too much overhead) with the ability for folks to turn it off (via make/cmake) if they don't want it or if they're concerned about performance?

@SeanTAllen
Copy link
Member

I think off by default is the right approach with documentation on the primitive pointing to build instructions to turn on.

@dipinhora
Copy link
Contributor Author

okey dokey.. makes sense..

@SeanTAllen
Copy link
Member

this needs a rebase

@dipinhora
Copy link
Contributor Author

rebased onto latest main

@SeanTAllen
Copy link
Member

needs another rebase

@SeanTAllen SeanTAllen added do not merge This PR should not be merged at this time and removed discuss during sync Should be discussed during an upcoming sync labels Jun 21, 2022
@jemc jemc removed the do not merge This PR should not be merged at this time label Jun 21, 2022
@SeanTAllen
Copy link
Member

This has outstanding changes requested.

During sync, Joe gave this a thumbs up.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jun 21, 2022
@SeanTAllen SeanTAllen added do not merge This PR should not be merged at this time and removed discuss during sync Should be discussed during an upcoming sync labels Jun 21, 2022
@SeanTAllen
Copy link
Member

Also needs a rebase against main

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jun 24, 2022
@SeanTAllen
Copy link
Member

@dipinhora drop me a note when we should check this out again.

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Jun 26, 2022
@dipinhora
Copy link
Contributor Author

@SeanTAllen yes, i'll drop a note when ready.. this took a bit of a back seat to the backpressure/systematic testing stuff (and in general i'd prefer to get those merged first anyways and then rebase/update this one accounting for all those changes)..

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jun 26, 2022
@dipinhora
Copy link
Contributor Author

argh, looks like a new comment automagically adds in the discuss during sync label so it's back again even though you just removed it... 8*/

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Jun 26, 2022
Runtime allocation tracking now also tracks the number
of heap allocations, the number of freed heap
allocations and the number of GC iterations via counters.

Additionally, there is now a way to check if runtime
memory allocation tracking is enabled or not via
`ifdef` statements in Pony code. This allows for some
useful validations for those folks concerned about
heap allocations in the critical path (i.e. if they
rely on the compiler's `HeapToStack` optimization pass
to convert heap allocations to stack allocations and
want to validate it is working correctly).
This commit enhances the runtime stats tracking that was previously
implemented under the `USE_MEMTRACK` and `USED_MEMTRACK_MESSAGES`
defines. The new defines are called `USE_RUNTIMESTATS` and
`USE_RUNTIMESTATS_MESSAGES`.

Runtime stats tracking tracks the following actor info:
* heap memory allocated
* heap memory used
* heap num allocated
* heap realloc counter
* heap alloc counter
* heap free counter
* heap gc counter
* system message processing cpu usage
* app message processing cpu usage
* garbage collection cpu usage
* messages sent counter
* system messages processed counter
* app messages processed counter

Runtime tracking tracks the following scheduler info:
* mutemap memory used
* mutemap memory allocated
* memory used for gc acquire/release actormaps and actors created
* memory allocated for gc acquire/release actormaps and actors created
* created actors counter
* destroyed actors counter
* actor system message processing cpu for all actor runs on the scheduler
* actor app message processing cpu for all actor runs on the scheduler
* actor garbage collection cpu for all actor runs on the scheduler
* scheduler message processing cpu usage
* scheduler misc cpu usage while waiting to do work
* memory used by inflight messages
* memory allocated by inflight messages
* number of inflight messages

This runtime stats tracking info has been exposed to pony programs as
part of the `runtime_info` package and an example `runtime_info` program
has been added to the `examples` directory.
@dipinhora dipinhora changed the title Enhance runtime memory allocation tracking Enhance runtime stats tracking Jul 10, 2022
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jul 10, 2022
@dipinhora
Copy link
Contributor Author

@SeanTAllen This is ready for review.

Comment on lines +1295 to +1304
static void platform_runtimestats(compile_t* c, reach_type_t* t, token_id cap)
{
FIND_METHOD("runtimestats", cap);
start_function(c, t, m, c->i1, &c_t->use_type, 1);

#if defined(USE_RUNTIMESTATS) || defined(USE_RUNTIMESTATS_MESSAGES)
bool runtimestats_enabled = true;
#else
bool runtimestats_enabled = false;
#endif
Copy link
Member

Choose a reason for hiding this comment

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

I made a comment in the sync call today that I'd eventually like to see this happen differently.

That is, the way this is written, it assumes that the USE_RUNTIMESTATS define is synchronized on both ponyc and libponyrt. What this implies is that in order to compile a program with runtime stats you need to use a completely different compiler instead of just linking to a different runtime.

I'd eventually like us to get to a place where we bundle both versions of the runtime (with and without stats) such that when compiling a program you can use a flag to switch it on or off.

Doesn't need to happen in this PR - we just want to make sure we create a followup ticket to capture that intended direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Ideally all of the various use options (

ponyc/Makefile

Lines 117 to 143 in a3b3bdf

$$(info Enabling use option: $1)
ifeq ($1,valgrind)
PONY_USES += -DPONY_USE_VALGRIND=true
else ifeq ($1,thread_sanitizer)
PONY_USES += -DPONY_USE_THREAD_SANITIZER=true
else ifeq ($1,address_sanitizer)
PONY_USES += -DPONY_USE_ADDRESS_SANITIZER=true
else ifeq ($1,undefined_behavior_sanitizer)
PONY_USES += -DPONY_USE_UNDEFINED_BEHAVIOR_SANITIZER=true
else ifeq ($1,coverage)
PONY_USES += -DPONY_USE_COVERAGE=true
else ifeq ($1,pooltrack)
PONY_USES += -DPONY_USE_POOLTRACK=true
else ifeq ($1,dtrace)
DTRACE ?= $(shell which dtrace)
ifeq (, $$(DTRACE))
$$(error No dtrace compatible user application static probe generation tool found)
endif
PONY_USES += -DPONY_USE_DTRACE=true
else ifeq ($1,scheduler_scaling_pthreads)
PONY_USES += -DPONY_USE_SCHEDULER_SCALING_PTHREADS=true
else ifeq ($1,systematic_testing)
PONY_USES += -DPONY_USE_SYSTEMATIC_TESTING=true
else ifeq ($1,memtrack)
PONY_USES += -DPONY_USE_MEMTRACK=true
else ifeq ($1,memtrack_messages)
PONY_USES += -DPONY_USE_MEMTRACK_MESSAGES=true
) would be easy to enable via cli arguments.

Copy link
Member

Choose a reason for hiding this comment

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

It's not easy to imagine at this time how we'd get all of those to be individually tweakable at Pony-program-compile-time unless we change how things work significantly in order to compile the runtime alongside the Pony program.

What I was thinking of in my previous comment would be to ship two versions of the runtime - one with extra stats enabled, and one without. We couldn't use that kind of approach with individually tweakable options unless we shipped an exponential number of runtimes with the Pony compiler releases.

A lot of those USE_* defines are more geared toward troubleshooting of the runtime by runtime developers (valgrind, sanitizers, etc), whereas I think a few of them (runtime stats, memtrack, pooltrack, dtrace) could be more helpful to application developers to instrument their application.

So I'm thinking it may be worthwhile to just lump a group of them together as a "with stats" version of the runtime and ship that. Then if some people really end up needing more granularity than that, they can custom build.

Copy link
Member

Choose a reason for hiding this comment

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

Another approach I've thought about in the past for this is seeing if it's possible to refactor these things such that they can be tweaked in the runtime bitcode individually, by rewriting certain instructions in the LLVM IR for the runtime bitcode, based on ponyc invocation flags. I did some brief work on this, but it got tricky because it's not enough to simply effect execution paths - a lot of these stats-related features also have an impact on memory layout (i.e. C ifdefs around struct field declarations), so I haven't worked further on that idea at this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i understand and agree with everything you've said the re: making use options controllable via ponyc and that it is not an easy or trivial thing to accomplish. i was simply stating that in an ideal scenario, they would all be selectable via ponyc as they can be useful for "power users" (along with folks working on/troubleshooting the runtime). Your idea around shipping multiple versions of the runtime and selecting between them based on compiler flags sounds like a great next step.

@ergl
Copy link
Member

ergl commented Jul 19, 2022

@dipinhora Can you add a note about the new --ponyprintstatsinterval flag to the release notes?

@dipinhora
Copy link
Contributor Author

@ergl added

@jemc
Copy link
Member

jemc commented Aug 2, 2022

@SeanTAllen - do you want to review again before we merge?

@SeanTAllen
Copy link
Member

I'm ok with no more review from me.

@SeanTAllen SeanTAllen merged commit 1c8f7b9 into ponylang:main Aug 3, 2022
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Aug 3, 2022
github-actions bot pushed a commit that referenced this pull request Aug 3, 2022
github-actions bot pushed a commit that referenced this pull request Aug 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - added Automatically add "Added" CHANGELOG entry on merge do not merge This PR should not be merged at this time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants