Last Comment Bug 799536 - Sync services are running on b2g
: Sync services are running on b2g
Status: RESOLVED FIXED
[MemShrink]
:
Product: Firefox OS
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Justin Lebar (not reading bugmail)
:
Mentors:
: 799549 (view as bug list)
Depends on: 800154
Blocks: slim-fast
  Show dependency treegraph
 
Reported: 2012-10-09 09:25 PDT by Justin Lebar (not reading bugmail)
Modified: 2012-10-11 07:20 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
fixed


Attachments
Patch, v1 (651 bytes, patch)
2012-10-09 09:46 PDT, Justin Lebar (not reading bugmail)
fabrice: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2012-10-09 09:25:44 PDT
I see e.g.

│  │  │  ├──────83,096 B (00.12%) -- compartment([System Principal], resource://services-sync/identity.js)

These compartments don't appear to be taking up a ton of memory, but we definitely should turn them off.  It's also possible they're using more memory than we think, due to message-manager overhead (i.e., these or other scripts could be causing bug 7980020).
Comment 1 Justin Lebar (not reading bugmail) 2012-10-09 09:44:56 PDT
> (i.e., these or other scripts could be causing bug 7980020).

bug 798002.

That sync is running appears to be intentional.  in b2g/confvars.sh, we have:

> MOZ_SERVICES_SYNC=1

I thought we weren't using sync in b2g v1?
Comment 2 Justin Lebar (not reading bugmail) 2012-10-09 09:46:08 PDT
Created attachment 669599 [details] [diff] [review]
Patch, v1
Comment 3 Justin Lebar (not reading bugmail) 2012-10-09 09:49:41 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/0eaac88ab86d
Comment 4 Justin Lebar (not reading bugmail) 2012-10-09 10:15:31 PDT
*** Bug 799549 has been marked as a duplicate of this bug. ***
Comment 5 Andrew Overholt [:overholt] (back Aug 31) 2012-10-09 10:46:33 PDT
Apparently we're not doing sync in v1.
Comment 6 Justin Lebar (not reading bugmail) 2012-10-09 10:47:14 PDT
I'll land in Aurora once I get some greenage on m-i.
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-10-09 18:34:26 PDT
https://hg.mozilla.org/mozilla-central/rev/0eaac88ab86d
Comment 8 Justin Lebar (not reading bugmail) 2012-10-10 08:24:02 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/08abc3599a33

We don't currently have bugzilla flags to indicate fixed-on-aurora, but we'll have them on Thursday.
Comment 9 Jonathan Griffin (:jgriffin) 2012-10-10 15:14:50 PDT
This change breaks Marionette because we use log4moz.js.
Comment 10 Gregory Szorc [:gps] 2012-10-10 15:17:40 PDT
Comment on attachment 669599 [details] [diff] [review]
Patch, v1

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

::: b2g/confvars.sh
@@ +16,5 @@
>  MOZ_OFFICIAL_BRANDING_DIRECTORY=b2g/branding/official
>  # MOZ_APP_DISPLAYNAME is set by branding/configure.sh
>  
>  MOZ_SAFE_BROWSING=
> +MOZ_SERVICES_SYNC=

For future reference, you should just delete the variable wholesale.
Comment 11 Justin Lebar (not reading bugmail) 2012-10-10 15:19:01 PDT
(In reply to Jonathan Griffin (:jgriffin) from comment #9)
> This change breaks Marionette because we use log4moz.js.

Not to be glib, but it seems like a bug in Marionette that it relies on a module from sync.

Anyway, I'm happy for you to back this out while you fix Marionette, so long as you'll put it back in once the dependency is resolved.
Comment 12 Justin Lebar (not reading bugmail) 2012-10-10 15:19:59 PDT
> For future reference, you should just delete the variable wholesale.

Should I also delete the other variables?  Or are there circumstances where "FOO=" is the right thing to do?
Comment 13 Gregory Szorc [:gps] 2012-10-10 15:27:46 PDT
Marionette does not rely on Sync. There are toolkit-like files in service/common (like log4moz.js) that Marionette relies on. The build system isn't smart enough to differentiate between services/common and services/sync. I'll code that up in bug 800154.

(In reply to Justin Lebar [:jlebar] from comment #12)
> > For future reference, you should just delete the variable wholesale.
> 
> Should I also delete the other variables?  Or are there circumstances where
> "FOO=" is the right thing to do?

It's a style thing. The default value for an unknown variable is undefined. It's safer to just not define a variable than to define one with an empty value, IMO.

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