tree logging improvements

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(7 attachments)

No description provided.
Attachment #8760331 - Flags: review?(yzenevich)
Attachment #8760331 - Flags: review?(yzenevich) → review+
Attachment #8760334 - Flags: review?(yzenevich) → review+
https://hg.mozilla.org/mozilla-central/rev/2b197e09ce5b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Status: RESOLVED → REOPENED
Flags: needinfo?(surkov.alexander)
Keywords: leave-open
Resolution: FIXED → ---
Attachment #8761160 - Flags: review?(yzenevich) → review+
Attachment #8761254 - Flags: review?(yzenevich) → review+
Comment on attachment 8761264 [details] [diff] [review]
part5: tree logging for single insertions

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

thanks

::: accessible/generic/DocAccessible.cpp
@@ +1819,5 @@
> +                    aContainer);
> +#endif
> +
> +#ifdef A11Y_LOG
> +    logging::TreeInfo("traversing an inserted node", logging::eVerbose,

nit: indentation.
Attachment #8761264 - Flags: review?(yzenevich) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1f370d930d5d4347c38bb99a5def3bee6d91bd5
Bug 1278294 - make all other tree loggings to log document address, part4, r=yzen
Assignee: nobody → surkov.alexander
Attachment #8761315 - Flags: review?(yzenevich)
I found it useful to add in the code when needed for debugging proposes.
Attachment #8762142 - Flags: review?(yzenevich)
Comment on attachment 8761315 [details] [diff] [review]
part6: isolate tree dumping into separate method for easier reuse

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

Looks good , thanks
Attachment #8761315 - Flags: review?(yzenevich) → review+
Comment on attachment 8762142 [details] [diff] [review]
part7: DOM tree logging

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

Looks good. Just a quick question: does it make sense to assign root->Get...() calls to a variable to avoid calling the method twice? Or it doesn't really affect performance?
Attachment #8762142 - Flags: review?(yzenevich) → review+
(In reply to Yura Zenevich [:yzen] from comment #19)
> Comment on attachment 8762142 [details] [diff] [review]
> part7: DOM tree logging
> 
> Review of attachment 8762142 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. Just a quick question: does it make sense to assign
> root->Get...() calls to a variable to avoid calling the method twice? Or it
> doesn't really affect performance?

if you call that logging function at all that will hurt your performance :) the logging is not about performance in general. Anyway those Get methods has to be inline methods, and it may be even faster to call them directly, as you don't have to create a local variable.
gotcha
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/46178702a3ca
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.