1224456 - TraceLogger: Make TLTextIdIsToggable(uint32_t id) more consistent

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[Wei Wu [:wuwei UTC+8]]

In tracelogger, all the events belong to LOG_ITEMS are not toggable. Current implementation treats LOG_ITEMS as the same as TREE_ITEMS, which may confuses users.

Comment 1

[Wei Wu [:wuwei UTC+8]]

Attachment: bug1224456-aggressive.patch

Aggressive version.

Comment 2

[Wei Wu [:wuwei UTC+8]]

Attachment: bug1224456-conservative.patch

Conservative version.

Comment 3

[Hannes Verschore [:h4writer]]

Log_items are also switchable. The log items aren't not that worked out that good yet and we don't have many of them.
But the ones that are toggable are:
- Bailout         (records every time a bailout happens, e.g. going from IonMonkey to Baseline due baked in assumption in IM)
- Invalidation    (same as bailout, but much heavier, since it also removes the IM code. This was incorrectly called an event before your patch.)
- Disable         (records every time we disable the tracelogger)
- Enable          (records every time we enable the tracelogger)

Special log items are. Only those are not toggable:
- Error
- Stop

I think you are looking at this code with only TraceLoggingGraph in your head.
- TraceLogging (in theory) doesn't make a distinction between log items and tree items. It will keep them in an incremental buffer one after the other. Where 1 item can be a log item or a tree item.
- TraceLoggingGraph makes a distinction between tree and log items. It separates the two from the TraceLogging buffer again. From the tree items it will construct a graph (with parent and children). And keep an incremental buffer for the log items.

Comment 4

[Wei Wu [:wuwei UTC+8]]

(In reply to Hannes Verschore [:h4writer] from comment #3)
> Log_items are also switchable. The log items aren't not that worked out that
> good yet and we don't have many of them.
> But the ones that are toggable are:
> - Bailout         (records every time a bailout happens, e.g. going from
> IonMonkey to Baseline due baked in assumption in IM)
> - Invalidation    (same as bailout, but much heavier, since it also removes
> the IM code. This was incorrectly called an event before your patch.)
> - Disable         (records every time we disable the tracelogger)
> - Enable          (records every time we enable the tracelogger)
> 
> Special log items are. Only those are not toggable:
> - Error
> - Stop
> 
> I think you are looking at this code with only TraceLoggingGraph in your
> head.
> - TraceLogging (in theory) doesn't make a distinction between log items and
> tree items. It will keep them in an incremental buffer one after the other.
> Where 1 item can be a log item or a tree item.
> - TraceLoggingGraph makes a distinction between tree and log items. It
> separates the two from the TraceLogging buffer again. From the tree items it
> will construct a graph (with parent and children). And keep an incremental
> buffer for the log items.

Thank you for the explanation! It helps me a lot. :)