1007618 - Tracelogger: Attribute increasing memory to TraceLogger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Hannes Verschore [:h4writer]]

Noticed this when tracelogging in the browser. Increasing of the available memory can take time. So let this be visible in the graphs it is TraceLogging itself taking that time;.

Comment 1

[Hannes Verschore [:h4writer]]

Attachment: Patch

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8419352 [details] [diff] [review]
Patch

Review of attachment 8419352 [details] [diff] [review]:
-----------------------------------------------------------------

As discussed on IRC, this is a temporary solution, as tracelogging should not be observable and change the behavior of the program. Hannes was mentioning that in the future, tracelogger should have another thread flushing the events tree so that we don't have to reallocate at all.

Is it worth adding the same thing in logTimestamp, or do timestamps occur not so often and thus don't need space growing? (same question applies for stack)

::: js/src/vm/TraceLogging.cpp
@@ +586,3 @@
>          uint64_t start = rdtsc() - traceLoggers.startupTime;
> +        if (!tree.ensureSpaceBeforeAdd()) {
> +            if (!flush()) {

nit: if (!tree.ensureSpaceBeforeAdd() && !flush()) {

Comment 3

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b63fa55fe464

The stack shouldn't grow too big and currently logTimestamp also. So there is not needed as much.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b63fa55fe464