Last Comment Bug 733861 - Use std::ostreams for profile stringification
: Use std::ostreams for profile stringification
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Gecko Profiler (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Markus Stange [:mstange]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-07 11:45 PST by Markus Stange [:mstange]
Modified: 2012-03-24 13:57 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (6.06 KB, patch)
2012-03-07 11:45 PST, Markus Stange [:mstange]
b56girard: review+
Details | Diff | Splinter Review

Description Markus Stange [:mstange] 2012-03-07 11:45:28 PST
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!)
Comment 1 Benoit Girard (:BenWa) 2012-03-07 12:03:55 PST
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 3 Ryan VanderMeulen [:RyanVM] 2012-03-22 16:23:21 PDT
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] 2012-03-23 10:35:27 PDT
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] 2012-03-23 10:49:20 PDT
(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] 2012-03-23 12:11:22 PDT
Thanks!

Pushed to inbound with that change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b15746f43a1
Comment 7 Ed Morley [:emorley] 2012-03-24 13:57:55 PDT
https://hg.mozilla.org/mozilla-central/rev/8b15746f43a1

Note You need to log in before you can comment on or make changes to this bug.