Maximum event ID is limited to 65535

[vertexodessa]

In case the application wants to define more than 65535 events, it will end up with following message (web UI):

Undefined event type
The file tried to reference an event it didn't define. Perhaps it's corrupted?
tracing-framework github, issues

I think this might be caused by the fact the wireId variable is 16-bit
(and possibly here)

[vertexodessa]

According to wtf-trace format documentation, chunk ID is 4b: "4b chunk id"..
Does it make sense to use 4 byte format by the default in cppbindings?
I'm trying to collect profiling data from a heavy application and 16 bits doesn't seem enough..

[stellaraccident]

Yes, that is likely the limit you are running into. I don't disagree, but it is going to be a fairly substantial change. Up until now, the most effective way we have used the tool is by whitelisting tracing locations. We've instrumented some pretty substantial JavaScript and c++ programs that way with an emphasis on balancing the amount of tracing so that it stays useful and doesn't disrupt performance too much (we most often use the tool for tracing real time systems). The automatic annotation case has different values, but honestly, we've never considered it. Optimizing the size of individual event records is part of that heritage. I wouldn't want to sacrifice the whitelisted/real-time case, but possibly a format option for jumbo trace files (4 byte events, 8 byte timestamps). Then a c++ copt to compile that way? There's a fair amount of type size amounting that would need to be audited and corrected.

…

On Feb 18, 2017 9:22 AM, "Ihor Ivlev" ***@***.***> wrote: According to wtf-trace format documentation <https://github.com/google/tracing-framework/blob/master/docs/wtf-trace.md#binary-1>, chunk ID is 4b: "4b chunk id".. Does it make sense to use 4 byte format by the default in cppbindings? I'm trying to collect profiling data from a heavy application and 16 bits doesn't seem enough.. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#585 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAeKgMlf0E_Bg0Kv8z6L9hRDSaPhbNaeks5rdyjHgaJpZM4MFKqg> .

[vertexodessa]

hi Stella, the code I pushed for review works for me with cppbindings.
I'm attaching a sample 100000-event file collected using the patch.
You'll have to rebuild the browser extention to see it
We can try to branch-off 32-bit event sizes. Otherwise I'll think of the build-time switch solution to make it possible to build both 16-bit and 32-bit versions.
Currently it's not clear for me how to make it possible to switch in the browser extension (.js part)
autosave.1.wtf-trace.zip

[vertexodessa]

it's not a problem to change the wireId size on C++ side using a build define. It's a little more complicated to handle this properly on JavaScript side.
To solve this problem, we can try to advance wtf.data.formats.BinaryTrace.VERSION to the next version, "11"; this version should have an additional field defining the wireId field size.
https://github.com/google/tracing-framework/blob/master/docs/wtf-trace.md#binary

- 4b magic number: 0xDEADBEEF
- 4b WTF version: wtf.version.getValue()
- 4b format version: wtf.data.formats.BinaryTrace.VERSION
- chunk 0 (file header)

becomes

- 4b magic number: 0xDEADBEEF
- 4b WTF version: wtf.version.getValue()
- 4b format version: wtf.data.formats.BinaryTrace.VERSION
- 4b wireId size (number of bits): wtf.data.formats.BinaryTrace.WIRE_ID_BITWIDTH
- chunk 0 (file header)

[benvanik]

The top-level answer is: if you are hitting the 65k event ID limit you are using events wrong :)
You should be parameterizing a small set of events instead of unique events for each instance.

The only case where hitting the limit would make sense would be if you are programmatically instrumenting every function with a unique event type and each function is using an event type slot. That's not what WTF is designed for, though, and you won't get meaningful timings out of it and the whole thing will likely fall over pretty fast. Instead, the wtf-calls format (https://github.com/google/tracing-framework/blob/master/docs/wtf-calls.md) was made to handle this specific case by having 4 byte IDs and much faster parsing logic for such a structure.

The doc is not great, so your best bet is probably the code:
https://github.com/google/tracing-framework/blob/master/bin/instrument.js#L201
https://github.com/google/tracing-framework/blob/master/src/wtf/db/sources/callsdatasource.js

you can have either a stream of just 4b function IDs that map to a function table, or 4b function ID + 4b arbitrary value (most often time) by setting the attributes to [{"name": "time", "units": "us"}].

As for the wireId in wtf-trace files: uint16 is a limit to keep binary size and in-memory size (of both recording and reading buffers) small (which is important due to JS heap size limits), but also a practical limit for what the tool is meant to handle: it is optimized for millions of events but a small set of event types. There are some very resource-heavy operations with event types, the largest and most obvious being that a new function is JIT'ed for each event. Throwing hundreds of thousands of dynamically generated functions at v8 is not going to end in a happy time.

If wtf-calls does not work for you, can you describe your use case?

[vertexodessa]

@benvanik
hi Ben.
Thank you so much for explanation.
At first, you're right, it turned out I was using the events wrong: I was creating a different event for each method call.

However, it worth noticing that JavaScript extension built for 32-bit wireId, is able to open .wtf-trace files collected with both 16-bit and 32-bit wireId cppbindings build. Looking deeper into that, we can see that internally in cppbindings, 32 bits are used to store the wireId variable (I assume "int" type is 32-bit in your build):
1 2 3 4

Does this look like something we should improve?

P.S. I haven't looked deeper at the JS side yet, but going to have a look soon. AFAIU that's where you're expecting to benefit from the memory usage optimization.
P.P.S. not sure if it will be easy to support multiple threads with .wtf-calls

[vertexodessa]

After looking further into JS code, I think it's possible to implement efficient handling for both 16- and 32-bit Wire ID variables after bumping a format version.
However according to Ben's comments and recent investigations(commented here) it's probably not of high priority as of now.
Not sure whether we should keep this open, please feel free to close the issue or just keep it for reference purposes.

Thanks!

[stellaraccident]

I do agree with Ben's comments and wouldn't want to change the defaults. If the viewer could support both and we had a compile time flag for the c++ bindings to enable wide event support, I could get behind that. I would be supportive of a PR that did that but would probably not prioritize doing the work myself.

…

On Mar 1, 2017 12:33 AM, "Ihor Ivlev" ***@***.***> wrote: After looking further into JS code, I think it's possible to implement efficient handling for both 16- and 32-bit Wire ID variables after bumping a format version. However according to Ben's comments and recent investigations(commented here) it's probably not of high priority as of now. Not sure whether we should keep this open, please feel free to close the issue or just keep it for reference purposes. Thanks! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#585 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAeKgPQpsUis76twGWH0xS1jLh8usqIqks5rhS1vgaJpZM4MFKqg> .

[vertexodessa]

Thank you Stella, I guess implementing this feature might take more time than I currently have; Considering the fact I have a workaround (the pull request above), and that it's much harder to reproduce when the framework used properly, I would prefer not to prioritize the issue as well (at least for now).
Please feel free to close issue or just keep it for reference purposes in case someone else will stumble on something similar.