Lightning not working because unpacked extension are not supported, remove unpack:true

RESOLVED FIXED in 6.2

Status

enhancement
--
major
RESOLVED FIXED
2 years ago
9 months ago

People

(Reporter: Paenglab, Assigned: jorgk)

Tracking

(Blocks 2 bugs, {perf})

Lightning 6.2
Dependency tree / graph

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
With bug 1404697 we have more small files which can degrade the startup performance if the profile is located on a network device or slow hard disks.

Maybe we need to package Lightning in one file to be faster.

Updated

2 years ago
Keywords: perf
Summary: Consider to compress Lightning with unpack: true → Consider to compress Lightning with unpack: true to help startup perf
Do you mean unpack:false? It is currently already true due to the former binary component
Reporter

Comment 2

2 years ago
Not tested, bur changing this to 'false' should do the trick: https://dxr.mozilla.org/comm-central/source/calendar/lightning/install.rdf#43. Not sure if this also would pack Lightning in the distribution folder.
I think it should. The only thing that comes to mind is that I believe window icons don't work with a packed xpi, but I could be mistaking. Do we have any measurements if this really brings a performance benefit? On a local disk I can imagine extracting a 5mb zip on startup might counter the performance benefit.

If the window icons work and this has a measurable performance benefit, fine with me.
Reporter

Comment 4

2 years ago
I think the window icons are not a problem as in earlier times all XPIs stayed packed and then the icons worked.

About the performance I can't say anything. I opened the bug on MakeMyDay's request.
MakeMyDay, do you have concrete measurements? I can imagine the speed gain over a network drive, but we should also keep the regular case in mind since it is more common.
Flags: needinfo?(makemyday)
Summary: Consider to compress Lightning with unpack: true to help startup perf → Consider to compress Lightning by removing unpack: true to help startup perf

Comment 6

2 years ago
Not offhand yet, but I recall several complaints for synced Windows profiles and network stored TB profiles back when dealing with the file sync issue at the time we started bundling Lightning. But as this bug about considering the switch, capturing some data first is the right thing to do.

A fairly easy test approach would be to measure startup time with a fresh profile (since we just care about the Lightning files here, not the other data in the TB profile) without additional calendars configured and comparing times with Lightning (a) disabled, (b) packed and (c) unpacked. The tests should be repeated several times each to get a stable measurement. For (b) and (c), the tested package should be over-installed if tested with Daily to make sure Lightning is taken from the user profile.

Maybe it would be helpful to do such tests in different machine setups with a local profile to get a grasp of a standard user's impact (Maybe a devs machine is not a good reference point here, since average users may have less powerful machines in terms of storage type, memory and cpu power).

Additionally, we would need to get the impact tested on a synced Windows user profile and a network stored TB profile to make sure that this has the expected benefit.

Probably we should provide two xpis here (with unpack true and false) and ask for some community support to do such measurement. For the latter scenario, we probably could ask on the enterprise list.
Flags: needinfo?(makemyday)
Assignee

Comment 7

Last year
Since support for "unpack" has just been removed here:
https://hg.mozilla.org/mozilla-central/rev/b8b11082ff88#l2.48 (bug 1444502)
you might as well just remove it.

Lightning, Gdata and two more on the C-C tree:
https://dxr.mozilla.org/comm-central/search?q=em%3Aunpack&redirect=false
Assignee

Comment 8

Last year
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8957889 - Flags: review?(philipp)
Comment on attachment 8957889 [details] [diff] [review]
1406499-unpack-true-removal.patch

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

r+ provided mozmill/jsbridge still works afterwards. It would be great if you could determine why mozmill/jsbridge needed this so we know it is safe to remove.
Attachment #8957889 - Flags: review?(philipp) → review+
Assignee

Comment 10

Last year
Well, M-C ignore this now and we have a Daily run with no errors. So this is just a clean-up to remove the option which is no longer supported.

Comment 11

Last year
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/6654b9108d7e
remove unpack:true from all extensions in C-C after removal of the feature in bug 1444502. r=philipp
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Assignee

Updated

Last year
Target Milestone: --- → 6.2
Assignee

Comment 12

Last year
That caused a heap of test failures in Calendar:
JavaScript error: resource://calendar/modules/calUtils.jsm, line 79: ReferenceError: __LOCATION__ is not defined.

I'm reverting the calendar changes.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 6.2 → ---
Assignee

Updated

Last year
Keywords: leave-open

Comment 13

Last year
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/50cd20fcaba7
Partly backed out changeset 6654b9108d7e (calendar parts) for causing test failures. a=backout
Severity: normal → major
Summary: Consider to compress Lightning by removing unpack: true to help startup perf → Lightning is not working because unpacked extension are not supported anymore (remove unpack:true)
Version: unspecified → Lightning 6.2
Summary: Lightning is not working because unpacked extension are not supported anymore (remove unpack:true) → Lightning not working because unpacked extension are not supported, remove unpack:true
I tested Lightning 6.2b4 / Thunderbird 60.0b4 (candidate build1 that should include dependent bug 905097) with unpack true removed. Lightning xpi can be installed but during the required restart Thunderbird crashes. Afterwards Lightning is non-functional.

[@ cal::getTimezoneService ] 
MOZ_CRASH(Could not load timezone service, brace yourself and prepare for crash)

https://crash-stats.mozilla.com/report/index/bp-742937b4-12e8-4e78-a512-7ba3d0180421
https://crash-stats.mozilla.com/report/index/bp-d0f5204f-1a68-456d-abed-455f00180421
Perhaps it is time to re-read the initial comment in https://bugzilla.mozilla.org/show_bug.cgi?id=401779 and fix this once and for all.

Updated

Last year
Blocks: 401779
bug 401779 really is not related to this breakage.
Blocks: tb-ltng-updateprob
No longer blocks: 401779
Assignee

Updated

Last year
Keywords: leave-open
Target Milestone: --- → 6.3

Comment 17

Last year
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/fa20f27405ec
remove unpack:true from Lightning after removal of the feature in bug 1444502. r=philipp
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → FIXED
Assignee

Comment 18

Last year
Comment on attachment 8957889 [details] [diff] [review]
1406499-unpack-true-removal.patch

Approval for the Calendar hunks only, the rest has already landed on TB 60.
Attachment #8957889 - Flags: approval-calendar-beta?(philipp)
Attachment #8957889 - Flags: approval-calendar-beta?(philipp) → approval-calendar-beta+
Assignee

Comment 19

Last year
Beta (TB 60 beta 6, Calendar 6.2):
https://hg.mozilla.org/releases/comm-beta/rev/b2d63636dca84397d55784504b141e450b6c8e2a
Target Milestone: 6.3 → 6.2

Updated

9 months ago
Blocks: 1202613
You need to log in before you can comment on or make changes to this bug.