733861 - Use std::ostreams for profile stringification

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Markus Stange [:mstange]]

Attachment: 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!)

Comment 1

[Benoit Girard (:BenWa)]

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.

Comment 2

[Markus Stange [:mstange]]

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

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Backed out due to making a bonfire of inbound. Try FTW :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/add2ed43cc05

Comment 4

[Markus Stange [:mstange]]

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?

Comment 5

[Mike Hommey [:glandium]]

(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

Comment 6

[Markus Stange [:mstange]]

Thanks!

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

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8b15746f43a1