Comment on attachment 8360504 [details] [diff] [review] v1 Review of attachment 8360504 [details] [diff] [review]: ----------------------------------------------------------------- Sorry I was bit distracted on IRC I should of given you a better answer. We should probably do something similar to ReadProfilerVars here rather then rewriting this code.

Comment on attachment 8360504 [details] [diff] [review] v1 Waiting on conversation before reviewing this.

Morphing this bug to cover profiler features as well.

Comment on attachment 8888973 [details] Bug 960153 - Add env var MOZ_PROFILER_STARTUP_FILTERS that lets you select which threads should be profiled during startup profiling. https://reviewboard.mozilla.org/r/159992/#review165428 ::: tools/profiler/core/platform.cpp:2080 (Diff revision 1) > + nsTArray<const char*> array; > + size_t len = strlen(aString); > + size_t currentElementStart = 0; > + aStorage = MakeUnique<char[]>(len + 1); > + // Iterate over all characters and split at commas. > + for (size_t i = 0; i < len; i++) { This works but I suspect it would be simpler if you strdup'd |aString| and then iterated over the copy, converting the commas to '\0' and recording the pointers in |array| as you go. ::: tools/profiler/core/platform.cpp:2105 (Diff revision 1) > > SharedLibraryInfo::Initialize(); > > + if (getenv("MOZ_PROFILER_HELP")) { > + PrintUsageThenExit(0); // terminates execution > + } You could move this above SharedLibraryInfo::Initialize().

Comment on attachment 8888974 [details] Bug 960153 - Factor out ParseFeaturesFromStringArray. https://reviewboard.mozilla.org/r/159994/#review165430 You sure you don't want to change nsIProfiler and the profiler extension to use a bitfield instead of this list-of-strings approach? :)

Comment on attachment 8888975 [details] Bug 960153 - Add env var MOZ_PROFILER_STARTUP_FEATURES that lets you select which features should be active for startup profiling. https://reviewboard.mozilla.org/r/159996/#review165432

Comment on attachment 8888973 [details] Bug 960153 - Add env var MOZ_PROFILER_STARTUP_FILTERS that lets you select which threads should be profiled during startup profiling. https://reviewboard.mozilla.org/r/159992/#review165428 > This works but I suspect it would be simpler if you strdup'd |aString| and then iterated over the copy, converting the commas to '\0' and recording the pointers in |array| as you go. I'm now using MakeUnique and PodCopy, and then iterate over the copied string. I didn't want to use strdup because then I would have had to use a UniquePtr<char,JS::FreePolicy> and somehow that seemed too complicated to me. > You could move this above SharedLibraryInfo::Initialize(). Done.

Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/50ef560f1f56 Add env var MOZ_PROFILER_STARTUP_FILTERS that lets you select which threads should be profiled during startup profiling. r=njn https://hg.mozilla.org/integration/autoland/rev/4ef452538efd Factor out ParseFeaturesFromStringArray. r=njn https://hg.mozilla.org/integration/autoland/rev/4b4ef581b92a Add env var MOZ_PROFILER_STARTUP_FEATURES that lets you select which features should be active for startup profiling. r=njn

https://hg.mozilla.org/mozilla-central/rev/50ef560f1f56 https://hg.mozilla.org/mozilla-central/rev/4ef452538efd https://hg.mozilla.org/mozilla-central/rev/4b4ef581b92a

Just putting this on your radar florian, since I recall you needing this capability.