@pfreixes @TimothyGu ... both issues fixed.

Thanks for the work done! Good to see this feature moving forward.

@cjihrig ... fixed...

btw, I intend for all the commits to be squashed into one when this lands.

Hi libuv members - @cjihrig @saghul @bnoordhuis and so on.

I would like to use the characteristics that this PR introduces, allowing the developers to make some profiling and getting statistics of the loop performance.

So, basically my question is: which are the plans for the v2 and specifically for that PR?

Thanks,

So, basically my question is: which are the plans for the v2 and specifically for that PR?

I would say that, most probably, v2 is not going to happen soon. See: #1597.

Would definitely like to know what else this needs before it can land. I'd definitely like to make sure it's part of 2.0, even if 2.0 is going to take a while.

Ping @cjihrig @saghul @bnoordhuis ... is there anything else needed to get this landed?

I haven't forgotten about this. I've been removing and landing things from the v2 PR backlog. It's down to 8 (with 2 ready to land very soon) from 14 two weeks ago. I've been trying to prioritize the things that are several years old already 😄

Awesome. I appreciate the work you are doing on getting things caught up. Just wanted to make sure there wasn't something else blocking this :)

I've been thinking: since Node.js uses Chromium's trace_events now, wouldn't it make sense to start using that in libuv too? I mean, this PR is mostly fine but ultimately it's to feed data into trace_events in a roundabout way.

That's the plan here, to be honest. Once integrated into node.js, I'll have a PR that kicks this data out to both the trace events stream and out to the perf API.

I suppose we could integrate the trace event macros directly into libuv but that would add a dependency that's really not strictly necessary and we would need to decide if it makes sense to pull in the complete machinery for outputting the trace event file.

I can not speak on their behalf but I guess that other libuv dependencies such as gevent, uvloop, u others will feel more comfortable working with the raw events provided by this MR.

need to decide if it makes sense to pull in the complete machinery for outputting the trace event file

Libuv is a library so no, that would be the responsibility of the embedder. Libuv just needs to make the events available in a way that's easily consumable.

other libuv dependencies such as gevent, uvloop, u others will feel more comfortable working with the raw events provided by this MR.

MR = PR? trace_events is in many ways 'rawer' than what this PR provides.

Well, I think either way we'd end up in roughly the same place but we could take an approach of invoking a callback at each collection point rather than setting a timestamp in a struct. Either way works for me, I'm more interested in getting the data out somehow than in any one specific way :-)

In other words, I'm happy to rework this PR in any way you think would be better @bnoordhuis :-)

Is not libuv a library used by many other technologies? Changes like that would suppose force these tecnologies to use a specific dependency to make usage of the tracing system?

If this is the case, I would prefer to keep the current aproach.

@cjihrig @santigimeno @saghul WDYT?

What exactly are the options on the table? I'm not really keen on the idea of integrating directly with trace_events or anything else from the Chromium universe if it can be avoided.

TBH I'm not familiar with the trace_events at all and the current proposal looks pretty generic which I like. Are there other advantages to of using trace_events apart from easier integration with node?

If I'm understanding @bnoordhuis correctly, the options on the table are:

For the loop stats:

1. Collect timestamps in a struct as the tick proceeds, invoke a callback once at the end of the tick (which is what this PR does)

2. Invoke a callback at each collection point (rather than grab the timestamp).

The difference between the two is that 1 requires a storage struct and a single callback per tick, where as 2 means we don't need to store timestamps but have multiple callbacks per tick.

For the threadpool stats, that would remain the same as implemented here.

@jasnell thanks for the explanation. Could you elaborate a little where trace_events come into play?

The only actual trace events integration here would be on the embedder-side. E.g. Node.js would register callbacks that would emit v8 trace events... Which we can easily do with either of the two approaches. 1 records the events after they happen at the end of the tick, 2 records the events as they happen. 2 would have the advantage of at least recording partial information should the process exit in the middle of the tick.

@jasnell thanks.

I think I like option 2 better.

Another advantage of trace_events is that we won't have to worry about ABI. An issue with this PR is that we can't easily add or change events without touching the struct.

Works for me. I'll get this reworked to drop the struct and use the 2nd approach.

So, ita just matter of change the pattern? Calling the callbacks at each event? If this is the case it wil suit for my proposals too

@bnoordhuis @santigimeno @saghul @cjihrig @evanlucas ....

Ok, so what I'm going to do is open a second PR that uses a more trace events like approach as discussed above. Rather than collecting timestamps into a struct it will invoke a trace callback as the loop progresses.

Before I get too far into the implementation, I want to solicit some opinions on the API because there are a couple ways I could do it.

Option 1: One generic trace callback

Internal function:

uv__trace(uv_loop_t* loop, uv_trace_type type, const char* name, ...);

In the loop:

for (...) { // in uv_run
  // ...
  uv__trace(loop, UV_TRACE_TYPE_START, "tick");

  uv__trace(loop, UV_TRACE_TYPE_START, "timers");
  count = uv_run_timers(...)
  uv__trace(loop, UV_TRACE_TYPE_END, "timers", "count", count);

  uv__trace(loop, UV_TRACE_TYPE_END, "tick");
  // ...
}

On the embedder side:

static void trace_cb(uv_trace_type* type, const char* name, void* data, va_list) {
  // ...
}

// ...
uv_trace_t config;
config.cb = trace_cb;
config.data = &loop;

uv_loop_config(&loop, UV_LOOP_TRACE, &config);
uv_run(...);

The key bit with this option is that a single callback is configured. Notice, however, the use of va_list in the callback signature. This mimics the V8 Trace Events macros that allow an arbitrary number of additional named arguments to be passed through. Unfortunately, doing it this way means the embedder will need to know how many named arguments and the types associated with those based on the name and type of the event being emitted. It would not be obvious in the API.

Therefore we have option 2: event-specific callbacks

On the libuv side:

for (...) { // in uv_run
  uv_trace_start(loop, UV_TRACE_PHASE_TICK);

  uv_trace_start(loop, UV_TRACE_PHASE_TIMERS);
  count = uv_run_timers(...);
  uv_trace_end(loop, UV_TRACE_PHASE_TIMERS, count);
  // ...
  uv_trace_end(loop, UV_TRACE_PHASE_TICK);
}

On the embedder side:

static void trace_start_cb(uv_trace_phase phase, void* data) {}
static void trace_end_cb(uv_trace_phase phase, size_t count, void* data) {}

uv_trace_t config;
config.start_cb = trace_start_cb;
config.end_cb = trace_end_cb;
config.data = &loop;

With this approach, the API is less ambiguous, more rigid, but also less flexible in terms of what data is passed through.

Which approach would each of you prefer?

There are other ways of doing this, of course... but the key question is how we should handle the arbitrary additional bits of information that should be included in the trace call. The poll event, for instance, should include the timeout, the various check|prepare|idle|timer end events should include the count, etc. If we go too rigid with it, then it would break ABI if we decide we need to add more info later. If we go too loose with it, the API becomes ambiguous.

Ok, see #1815 for an alternative take based around a trace events model.

Closing in favor of #1815