Last Comment Bug 771056 - Profile dump (temporary) path should not be hardcoded
: Profile dump (temporary) path should not be hardcoded
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Gecko Profiler (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-05 00:15 PDT by Mike Hommey [:glandium]
Modified: 2012-07-10 15:47 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Use NS_OS_TEMP_DIR to save SPS profile data (2.75 KB, patch)
2012-07-09 10:10 PDT, Mike Hommey [:glandium]
bgirard: review+
Details | Diff | Splinter Review
Get profile dumps from NS_OS_TEMP_DIR location on android and b2g (3.25 KB, patch)
2012-07-09 10:13 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Get profile dumps from NS_OS_TEMP_DIR location on android and b2g (3.34 KB, patch)
2012-07-09 11:18 PDT, Mike Hommey [:glandium]
bgirard: review+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2012-07-05 00:15:06 PDT
Instead, the directory service should be used (probably requesting for NS_OS_TEMP_DIR).
Comment 1 Mike Hommey [:glandium] 2012-07-09 10:10:16 PDT
Created attachment 640259 [details] [diff] [review]
Use NS_OS_TEMP_DIR to save SPS profile data

This requires a change to the addon that I'll attach here too.
Comment 2 Mike Hommey [:glandium] 2012-07-09 10:13:06 PDT
Created attachment 640260 [details] [diff] [review]
Get profile dumps from NS_OS_TEMP_DIR location on android and b2g
Comment 3 Benoit Girard (:BenWa) 2012-07-09 10:34:46 PDT
Comment on attachment 640260 [details] [diff] [review]
Get profile dumps from NS_OS_TEMP_DIR location on android and b2g

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

::: lib/main.js
@@ +522,5 @@
>                              // now pull the fennec libs themselves
>                              cmdRunnerModule.runCommand("/bin/bash -l -c 'rm tmp/symbol.apk'", function (r) {});
>                              cmdRunnerModule.runCommand(
> +                                "/bin/bash -l -c '" +
> +                                ["adb pull " + path + "/profile_0_" + processInfo.pid + ".txt /tmp/fennec_profile.txt" for each (path in processInfo.savePath)].join('||') + ";" +

I really don't like the use for [] for each join. Can we write this code to be simpler?
Comment 4 Benoit Girard (:BenWa) 2012-07-09 10:35:26 PDT
Comment on attachment 640259 [details] [diff] [review]
Use NS_OS_TEMP_DIR to save SPS profile data

I was going to ask what directory they would end up in but the next patch answers my question.
Comment 5 Mike Hommey [:glandium] 2012-07-09 11:18:22 PDT
Created attachment 640300 [details] [diff] [review]
Get profile dumps from NS_OS_TEMP_DIR location on android and b2g
Comment 6 Benoit Girard (:BenWa) 2012-07-09 11:28:28 PDT
Why are you no longer checking /sdcard? Couldn't you just moved the code to a helper function where it could of been written cleanly?
Comment 7 Mike Hommey [:glandium] 2012-07-09 11:30:50 PDT
(In reply to Benoit Girard (:BenWa) from comment #6)
> Why are you no longer checking /sdcard?

/sdcard is still checked.
Comment 8 Benoit Girard (:BenWa) 2012-07-09 11:35:49 PDT
Comment on attachment 640300 [details] [diff] [review]
Get profile dumps from NS_OS_TEMP_DIR location on android and b2g

Right. I see it now. I'll land this change when you land the first on m-c.
Comment 9 Mike Hommey [:glandium] 2012-07-09 11:38:49 PDT
(In reply to Benoit Girard (:BenWa) from comment #8)
> Comment on attachment 640300 [details] [diff] [review]
> Get profile dumps from NS_OS_TEMP_DIR location on android and b2g
> 
> Right. I see it now. I'll land this change when you land the first on m-c.

It would make more sense to do it the other way around. The addon change explicitely keeps backwards compatibility with the current m-c behaviour.
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-07-10 15:47:25 PDT
https://hg.mozilla.org/mozilla-central/rev/cdd36f5c088b

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