Closed Bug 1043324 Opened 10 years ago Closed 6 years ago

Implement nsIB2GInfoReporter to send b2g-info to WebIDE and break reliance on parsing shell script output.

Categories

(Firefox OS Graveyard :: GonkIntegration, defect, P3)

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: huseby, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [c=devtools p= s= u=])

Attachments

(1 file)

49 bytes, text/x-github-pull-request
dhylands
: review-
Details | Review
The realtime data gathering in the WebIDE requires getting the results of b2g-info.  Currently it is using fragile parsing code to extract the data from the output of b2g-info.  This bug is to add JSON output to b2g-info to eliminate the fragility in the parsing.
Attached file github pull request
this pull request adds JSON output to b2g-info
Attachment #8461497 - Flags: review?(mwu)
Attachment #8461497 - Flags: feedback?(janx)
Attachment #8461497 - Flags: feedback?(dhylands)
Blocks: 1043378
Why isn't webide using a remote debugging actor instead?
Comment on attachment 8461497 [details] [review]
github pull request

Passing review to dhylands, but I agree with fabrice that this seems a bit weird. WebIDE should be reading from the files that b2g-info uses to obtain its information, not b2g-info directly. b2g-info wasn't meant to be a standard interface, but a lot of the files that b2g-info reads from are standard Linux interfaces.
Attachment #8461497 - Flags: review?(mwu)
Attachment #8461497 - Flags: review?(dhylands)
Attachment #8461497 - Flags: feedback?(dhylands)
Comment on attachment 8461497 [details] [review]
github pull request

Code needs lots of cleanup to meet coding standards. So r- for now.

I tend to agree with mwu. If you can launch b2g-info you should be able to read the same /proc files that b2g-info reads.

b2g-info was just intended to be a command line helper since b2g-ps was written in bash, and could be a bit on the slow side.

I don't think that b2g-info adds any real value to the information from /proc, it just summarizes it into nice readable form (which outputting as json basically throws away). Since you seem to want the raw information, it seems like you should go directly to the source.
Attachment #8461497 - Flags: review?(dhylands) → review-
Alrighty, let me re-write this as a remote debugging actor then.  Thanks for the review.  Dave, I'm most interested in where I violated the coding standards so I can learn not to do that in the future.  I was mostly mimicking the style of the existing code.
Status: NEW → ASSIGNED
Flags: needinfo?(dhylands)
Dave, nvrmnd, I see your code review details in the github pull request.  Thanks for the feedback.
And the coding style guideline is here:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style
Flags: needinfo?(dhylands)
Comment on attachment 8461497 [details] [review]
github pull request

Thanks for working on this! We'll need something like this for WebIDE's Monitor (a graphing tool to track device performance).

For a bit of context, see bug 1020288 comment 33 where I incorporated your ideas about a devtools actor that does the same thing as `b2g-info`.
Attachment #8461497 - Flags: feedback?(janx)
After discussing this further with dhylands, I decided that the best way to solve the problem is to implement a B2GInfoReporter component in gecko.  That way the information can be sent to the WebIDE using the remote debugging protocol directly and we won't rely on parsing the output of b2g shell scripts.

I'm renaming this bug to reflect this decision.
Keywords: perf
Summary: Add JSON output to b2g-info → Implement nsIB2GInfoReporter to send b2g-info to WebIDE and break reliance on parsing shell script output.
Whiteboard: [c=devtools p= s= u=]
What data will be tracked by this B2GInfoReporter component?
Priority: -- → P3
Depends on: monitor
Need info per comment 10, but I guess at first USS of all processes, and in the future maybe CPU usage and Network up/download. Dave, any updates on this?

Also, is `b2g-info -j` still happening? If so, we need a separate bug for it. If not, we should close bug 1043378.
Flags: needinfo?(dhuseby)
b2g-info -j is *not* happening.  the correct way is to have an nsIB2GInfoReporter collecting and reporting directly.  I was busy with blackhat and defcon last week and now I've got a 2.0 blocker on my plate I'm working.  So this is a little ways down my priority list.

My plan was to replace the current execution and parsing of the b2g-info output.  IIRC, that's just USS of all of the processes ATM.  So I'm planning just to implement that for now to keep the scope small.
Flags: needinfo?(dhuseby)
Assignee: huseby → nobody
Firefox OS is not being worked on
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: