Use std::ostreams for profile stringification

RESOLVED FIXED in mozilla14

Status

()

Core
Gecko Profiler
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 603807 [details] [diff] [review]
patch

This makes the code shorter, easier to read, and a lot faster too (at least in my tests, probably due to less temporary strings).

This patch also includes the one from bug 726369. (The compiler didn't allow me to assign to entry.mReadPos of a const entry. Good thing!)
Attachment #603807 - Flags: review?(bgirard)
Comment on attachment 603807 [details] [diff] [review]
patch

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

Awesome. We started off with something really primitive because we planned on handling saving within a signal but this is no longer a requirement.

::: tools/profiler/TableTicker.cpp
@@ +540,5 @@
> +
> +std::ostream& operator<<(std::ostream& stream, const ProfileEntry& entry)
> +{
> +  if (entry.mTagName == 'r') {
> +    stream << entry.mTagName << "-" << std::fixed << entry.mTagFloat << std::endl;

Maybe we should set fixed once when we create the stream?

std::endl has an implicit flush, let's use '\n' instead.
Attachment #603807 - Flags: review?(bgirard) → review+
(Assignee)

Comment 2

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f42ea2a158e4
Backed out due to making a bonfire of inbound. Try FTW :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/add2ed43cc05
(Assignee)

Comment 4

6 years ago
Pushed a fixed version to try:
https://tbpl.mozilla.org/?tree=Try&rev=3bc8ea6b8409

Linux opt builds complain:
> TEST-UNEXPECTED-FAIL | | We don't want these libstdc++ symbols to be used:
> 0000000000000000      DF *UND*	0000000000000208  GLIBCXX_3.4.9 _ZNSo9_M_insertIPKvEERSoT_

Mike, what should I do here?
(In reply to Markus Stange from comment #4)
> Pushed a fixed version to try:
> https://tbpl.mozilla.org/?tree=Try&rev=3bc8ea6b8409
> 
> Linux opt builds complain:
> > TEST-UNEXPECTED-FAIL | | We don't want these libstdc++ symbols to be used:
> > 0000000000000000      DF *UND*	0000000000000208  GLIBCXX_3.4.9 _ZNSo9_M_insertIPKvEERSoT_
> 
> Mike, what should I do here?

diff --git a/build/stdc++compat.cpp b/build/stdc++compat.cpp
--- a/build/stdc++compat.cpp
+++ b/build/stdc++compat.cpp
@@ -59,9 +59,7 @@ namespace std {
     template ostream& ostream::_M_insert(unsigned long);
     template ostream& ostream::_M_insert(long long);
     template ostream& ostream::_M_insert(unsigned long long);
-#ifdef DEBUG
     template ostream& ostream::_M_insert(const void*);
-#endif
     template ostream& __ostream_insert(ostream&, const char*, streamsize);
     template istream& istream::_M_extract(double&);
 #endif
(Assignee)

Comment 6

6 years ago
Thanks!

Pushed to inbound with that change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b15746f43a1
Target Milestone: --- → mozilla14

Comment 7

6 years ago
https://hg.mozilla.org/mozilla-central/rev/8b15746f43a1
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.