Re: Always brace your ifs
On 17:37, Sat, 22 Feb, L. David Baron wrote: On Saturday 2014-02-22 15:57 -0800, Gregory Szorc wrote: On Feb 22, 2014, at 8:18, Kyle Huey m...@kylehuey.com wrote: If you needed another reason to follow the style guide: https://www.imperialviolet.org/2014/02/22/applebug.html Code coverage would have caught this as well. The time investment into 100% line and branch coverage is debatable. But you can't argue that code coverage has its place, especially for high-importance code such as crypto. AFAIK, our automation currently does not collect code coverage from any test suite. Should that change? There was some automation running code coverage reports (with gcov, I think) for at least reftests + mochitests for an extended period of time; I found it useful for improving style system test coverage while it was running. I'm not sure how strong our commitment is to keeping such tests running, though; I frequently have to defend the tests against people who want to disable them because they take a long time (i.e., a long time for a single test file, which sometimes leads the tests to approach the per-file timeouts on slow VMs) or because they happen to exhibit the latest JIT crash frequently because they run a lot of code. I'm worried we're moving to a model where tests need to have active defenders to keep them running (even though that isn't how features on the Web platform work), because we blame the old test rather than the new regression. Tests need owners just like any other piece of code. One of the big problems with the previous code coverage reports was that they were failing to run, and nobody was stepping up to fix them. When we're resource constrained, it's a big waste of resources to run things that are broken and nobody is working on fixing. Slow tests are a slightly different issue. If you're adding a test that takes 60s to run, you're saying you think it's important enough to make all other developers wait another minute for their test runs to complete locally. You're saying it's worthwhile to spend an extra minute per push per platform, to delay all future landings and merges by an extra minute per push. At ~249 pushes per day and at least 18 test platforms, you're adding at least 74 hours of additional machine time per day. I know the cost of *not* testing code is even higher than this! It's even more expensive and painful to track down regressions after the fact. This doesn't mean we can't put more effort into writing efficient tests. Cheers, Chris signature.asc Description: Digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Fixing build warnings [was: Re: Always brace your ifs]
On 24/02/2014 05:54, Daniel Holbert wrote: On 02/22/2014 12:26 PM, Hubert Figuière wrote: Now we talk about enabling more warning, yet Mozilla codebase is far from building warning free Maybe we should start with that first? FWIW, I (and others) have been working on that, as a side project, for a while now, and I think we're actually in pretty good shape right now. Well done. We currently have only 100-200 build warnings[1], if you filter out warnings from third-party libraries that we import (e.g. cairo, skia, protobuf, ICU, various media codecs). By the way, do you have any plan to do the same with these libraries and forward the patches upstream? Sylvestre ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Fixing build warnings [was: Re: Always brace your ifs]
On 02/24/2014 08:25 AM, Sylvestre Ledru wrote: By the way, do you have any plan to do the same with these libraries and forward the patches upstream? I don't have concrete plans to do this, but others are welcome to! It's often more difficult (with less immediate benefit) to fix warnings in third-party code, since: a) We try to avoid directly modifying third-party code in m-c, since any such patches would be clobbered on the next library-update. So we may not be able to directly fix our in-tree copy of the code, unless it's really important. b) In some cases, the lines of code in question might have already been fixed (or even removed) upstream, so no upstream patch ends up being needed. c) In cases where the bug *does* exist upstream, you may end up having to wait a while until Mozilla imports a new version of the library (with your warning-fix), for mozilla-central to get the fix. d) In the meantime, it's possible that upstream might have introduced a bunch of *new* build warnings, which we only discover when we import the new version. So, that can make it an uphill battle. ~Daniel ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Fixing build warnings [was: Re: Always brace your ifs]
On 2014-02-24 9:21 AM, Daniel Holbert wrote: a) We try to avoid directly modifying third-party code in m-c, since any such patches would be clobbered on the next library-update. So we may not be able to directly fix our in-tree copy of the code, unless it's really important. FWIW in the media subtree we maintain a separate copy of patches we've applied since the last import. An 'update' script remembers what files need to be copied from upstream and applies the patches, which helps a lot if upstream doesn't change much, and helps when checking if specific issues have been fixed. -r ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Fixing build warnings [was: Re: Always brace your ifs]
On 2/23/14, 8:54 PM, Daniel Holbert wrote: We currently have only 100-200 build warnings[1], if you filter out warnings from third-party libraries that we import (e.g. cairo, skia, protobuf, ICU, various media codecs). On my machine, Firefox for OS X has 284 warnings, but only 24 are from Mozilla code. :) If we think getting to 0 warnings is useful (I do), we might consider selectively disabling warnings in third-party libraries (with a comment pointing to an upstream bug report). For example, 51 out of 51 -Wtautological-constant-out-of-range-compare warnings are from cairo. chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of MOZ_ARRAY_LENGTH for static constants?
On 2/23/14, 4:05 PM, Neil wrote: Both ArrayLength and MOZ_ARRAY_LENGTH are typesafe when compiled as C++, however ArrayLength has the disadvantage that it's not a constant expression in MSVC. In unoptimised builds this is direly slow as the templated function does not even get inlined, but even in optimised builds the MSVC compiler is unable to completely optimise away static variables. In particular, the variable is treated as if it is forward-declared: the value is fetched from memory in each function that uses it. (In my simple test there were enough registers to avoid fetching the value more than once, but I don't know what happens if this is not the case. And at least the optimiser was able to avoid creating any static constructors.) Would it therefore be preferable to use MOZ_ARRAY_LENGTH in such cases? To avoid developers and reviewers from having to remember special cases, maybe MOZ_ARRAY_LENGTH should just be the standard everywhere. chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: We live in a memory-constrained world
On 2/22/14, 1:39 PM, Nicholas Nethercote wrote: On Sat, Feb 22, 2014 at 11:22 PM, Andreas Gal andreas@gmail.com wrote: So, I'm wondering how much effort we should put in reducing the number of ChromeWorkers. We should continue to use JS in Chrome where it makes sense. Its often easier and faster to write some functionality in JS (and sometimes also safer), and it tends to be more compact when we ship it. In addition to purging caches and stopping idle workers the JS team is also working on making workers (and JS) more memory efficient in general. Naveed might want to chime in here. Brian Hackett's been making great progress in reducing the overhead of workers. https://bugzilla.mozilla.org/show_bug.cgi?id=964059 and https://bugzilla.mozilla.org/show_bug.cgi?id=964057 both landed this week, reducing the overhead of a trivial worker from ~1 MiB to ~0.2 MiB. And https://bugzilla.mozilla.org/show_bug.cgi?id=864927 is the meta-bug, which has ideas for further improvements. Naveed told me that bug 941783 is meta-bug for all B2G DOM worker optimizations, beyond just memory reduction. He is preparing a bug backlog to prioritize those issues. chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: We live in a memory-constrained world
On 2/21/2014 5:40 PM, Brian Smith wrote: On Fri, Feb 21, 2014 at 1:38 PM, Nicholas Nethercote n.netherc...@gmail.com wrote: Optimizations that wouldn't have been worthwhile in the desktop-only days are now worthwhile. For example, an optimization that saves 100 KiB of memory per process is pretty worthwhile for Firefox OS. Do you mean 100KiB per child process? How worthwhile is it to cut 100KiB from the parent process? I ask because we have various caches that we use for TLS security (TLS session cache, HSTS, OCSP response cache), and I'd like to know how much memory we can can budge for these features, given that they all affect only a single process (the parent). In some cases, we can throw the information away and re-retrieve and/or recompute it, but only at a cost of reduced security and/or significantly reduced performance. In some cases, we could try to leave the data on disk and avoid loading it into memory, but then we run into main-thread-I/O and/or socket-thread-disk-I/O, both of which are important to avoid for responsiveness/perf reasons. Also, I am concerned that, AFAICT, no TLS-related testing is being done for AWSY. Thus, a lot of my work isn't being measured there. Who is the best person to talk to about getting TLS functionality added to AWSY? Not sure if these are what you are referring to, but I see nss security PORT_Alloc_Util() pretty consistently in parent process DMD. In a recent DMD report I saw it accounting for ~1.2MB over a number of different invocations. Ben ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Visual Studio Project Generation
On 2/21/2014 7:31 AM, Yuan Xulei(袁徐磊) wrote: That's really good! I'm looking forward to eclipse project generation feature. +1 -hb- PS: Gregory, thanks for your work on VS project generation! It's a pretty good stuff. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Rendering meeting, Monday 5:30pm PDT (the later time)
The Rendering meeting is about all things Gfx, Image, Layout, and Media. It takes place every second Monday, alternating between 2:30pm PDT and 5:30pm PDT. The next meeting will take place today, Monday, February 24th at 5:30 PM US/Pacific Please add to the agenda: https://wiki.mozilla.org/Platform/GFX/2014-February-24 San Francisco - Monday, 5:30pm Winnipeg - Monday, 7:30pm Toronto - Monday, 8:30pm GMT/UTC - Tuesday, 1:30 Paris - Tuesday, 2:30am Taipei - Tuesday, 9:30am Brisbane - Tuesday, 11:30am Auckland - Tuesday, 12:30pm http://arewemeetingyet.com/Toronto/2014-02-24/20:30/Rendering%20Meeting Video conferencing: Vidyo room Graphics (9366) https://v.mozilla.com/flex.html?roomdirect.htmlkey=FILzKzPcA6W2 Phone conferencing: +1 650 903 0800 x92 Conf# 99366 +1 416 848 3114 x92 Conf# 99366 +1 800 707 2533 (pin 369) Conf# 99366 -- - Milan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Including Adobe CMaps
PDF.js plans to soon start including and using Adobe CMap files for converting character codes to character id’s(CIDs) and mapping character codes to unicode values. This will fix a number of bugs in PDF.js and will improve our support for Chinese, Korean, and Japanese(CJK) documents. I wanted to inform dev-platform because there are quite a few files and they are large. The files are loaded lazily as needed so they shouldn’t affect the size of Firefox when running, but they will affect the installation size. There are 168 files with an average size of ~40KB, and all of the files together are roughly: 6.9M 2.2M when gzipped http://sourceforge.net/adobe/cmap/wiki/Home/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Including Adobe CMaps
Is this something we could load dynamically and offline cache? Andreas Sent from Mobile. On Feb 24, 2014, at 23:41, Brendan Dahl bd...@mozilla.com wrote: PDF.js plans to soon start including and using Adobe CMap files for converting character codes to character id's(CIDs) and mapping character codes to unicode values. This will fix a number of bugs in PDF.js and will improve our support for Chinese, Korean, and Japanese(CJK) documents. I wanted to inform dev-platform because there are quite a few files and they are large. The files are loaded lazily as needed so they shouldn't affect the size of Firefox when running, but they will affect the installation size. There are 168 files with an average size of ~40KB, and all of the files together are roughly: 6.9M 2.2M when gzipped http://sourceforge.net/adobe/cmap/wiki/Home/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Including Adobe CMaps
On Mon, Feb 24, 2014 at 3:01 PM, Andreas Gal andreas@gmail.com wrote: Is this something we could load dynamically and offline cache? Andreas Sent from Mobile. On Feb 24, 2014, at 23:41, Brendan Dahl bd...@mozilla.com wrote: PDF.js plans to soon start including and using Adobe CMap files for converting character codes to character id's(CIDs) and mapping character codes to unicode values. This will fix a number of bugs in PDF.js and will improve our support for Chinese, Korean, and Japanese(CJK) documents. I wanted to inform dev-platform because there are quite a few files and they are large. The files are loaded lazily as needed so they shouldn't affect the size of Firefox when running, but they will affect the installation size. There are 168 files with an average size of ~40KB, and all of the files together are roughly: 6.9M 2.2M when gzipped http://sourceforge.net/adobe/cmap/wiki/Home/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform Alternatively, is this data duplicated in ICU? - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Including Adobe CMaps
On 2014-02-24 2:41 PM, Brendan Dahl wrote: There are 168 files with an average size of ~40KB, and all of the files together are roughly: 6.9M 2.2M when gzipped IIRC mupdf was able to save significant space by pre-parsing the files. Their code for that is GPL (and oriented toward compiling-in the tables as C code) but it might be worth a look. http://git.ghostscript.com/?p=mupdf.git;a=blob;f=scripts/cmapdump.c 4.2Mresources/cmaps/ 1008K build/debug/pdf/pdf-cmap-table.o -r ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Including Adobe CMaps
On Mon, Feb 24, 2014 at 3:01 PM, Andreas Gal andreas@gmail.com wrote: Is this something we could load dynamically and offline cache? That should be possible. The CMap name is in the PDF so Firefox could download it on demand. Also, if the user has acrobat, the CMaps are already on their machine. On Feb 24, 2014, at 23:41, Brendan Dahl bd...@mozilla.com wrote: PDF.js plans to soon start including and using Adobe CMap files for converting character codes to character id's(CIDs) and mapping character codes to unicode values. This will fix a number of bugs in PDF.js and will improve our support for Chinese, Korean, and Japanese(CJK) documents. I wanted to inform dev-platform because there are quite a few files and they are large. The files are loaded lazily as needed so they shouldn't affect the size of Firefox when running, but they will affect the installation size. There are 168 files with an average size of ~40KB, and all of the files together are roughly: 6.9M 2.2M when gzipped http://sourceforge.net/adobe/cmap/wiki/Home/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Including Adobe CMaps
It’s certainly possible to load dynamically. Do we currently do this for any other Firefox resources? From what I’ve seen, many PDF’s use CMaps even if they don’t necessarily have CJK characters, so it may just be better to include them. FWIW both Popper and Mupdf embed the CMaps. Brendan On Feb 24, 2014, at 3:01 PM, Andreas Gal andreas@gmail.com wrote: Is this something we could load dynamically and offline cache? Andreas Sent from Mobile. On Feb 24, 2014, at 23:41, Brendan Dahl bd...@mozilla.com wrote: PDF.js plans to soon start including and using Adobe CMap files for converting character codes to character id's(CIDs) and mapping character codes to unicode values. This will fix a number of bugs in PDF.js and will improve our support for Chinese, Korean, and Japanese(CJK) documents. I wanted to inform dev-platform because there are quite a few files and they are large. The files are loaded lazily as needed so they shouldn't affect the size of Firefox when running, but they will affect the installation size. There are 168 files with an average size of ~40KB, and all of the files together are roughly: 6.9M 2.2M when gzipped http://sourceforge.net/adobe/cmap/wiki/Home/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Including Adobe CMaps
My assumption is that certain users only need certain CMaps because they tend to read only documents in certain languages. This seems like something we can really optimize and avoid ahead-of-time download cost for. The fact that we don’t do this yet doesn’t seem like a good criteria. There is a lot of good things we aren’t doing yet. You can be the first to change that on this particular topic, if it technically makes sense. Andreas On Feb 25, 2014, at 1:27 AM, Brendan Dahl bd...@mozilla.com wrote: It’s certainly possible to load dynamically. Do we currently do this for any other Firefox resources? From what I’ve seen, many PDF’s use CMaps even if they don’t necessarily have CJK characters, so it may just be better to include them. FWIW both Popper and Mupdf embed the CMaps. Brendan On Feb 24, 2014, at 3:01 PM, Andreas Gal andreas@gmail.com wrote: Is this something we could load dynamically and offline cache? Andreas Sent from Mobile. On Feb 24, 2014, at 23:41, Brendan Dahl bd...@mozilla.com wrote: PDF.js plans to soon start including and using Adobe CMap files for converting character codes to character id's(CIDs) and mapping character codes to unicode values. This will fix a number of bugs in PDF.js and will improve our support for Chinese, Korean, and Japanese(CJK) documents. I wanted to inform dev-platform because there are quite a few files and they are large. The files are loaded lazily as needed so they shouldn't affect the size of Firefox when running, but they will affect the installation size. There are 168 files with an average size of ~40KB, and all of the files together are roughly: 6.9M 2.2M when gzipped http://sourceforge.net/adobe/cmap/wiki/Home/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: We live in a memory-constrained world
On 02/21/2014 02:57 PM, Nicholas Nethercote wrote: On Sat, Feb 22, 2014 at 9:40 AM, Brian Smith br...@briansmith.org wrote: How worthwhile is it to cut 100KiB from the parent process? We don't have a fixed memory budget per se. If it's easy and doesn't have bad side-effects, do it. Otherwise... use your judgment. Having a Firefox OS device to test on helps with evaluating such things. Also, I am concerned that, AFAICT, no TLS-related testing is being done for AWSY. Thus, a lot of my work isn't being measured there. Who is the best person to talk to about getting TLS functionality added to AWSY? John Schoenick maintains AWSY. AWSY's current test is dead simple, and doesn't test many modern features: https://github.com/Nephyrin/MozAreWeSlimYet/blob/master/mozmill_endurance_test/test1.js I don't have the time or probably qualifications to design something better than TP5 -- but if someone wants to tackle making a better set of test pages, I think we'd gladly use it on AWSY. Ideally we'd be replaying network sessions of things like browsing a page, interacting with gmail, etc.. Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: We live in a memory-constrained world
Nick, do we run any kind of automated leak checking through valgrind? I found bug 976363 today, it's something that any automated leak detection tool should be able to catch. Thanks! Ehsan On 2/21/2014, 4:38 PM, Nicholas Nethercote wrote: Greetings, We now live in a memory-constrained world. By we, I mean anyone working on Mozilla platform code. When desktop Firefox was our only product, this wasn't especially true -- bad leaks and the like were a problem, sure, but ordinary usage wasn't much of an issue. But now with Firefox on Android and particularly Firefox OS, it is most definitely true. In particular, work is currently underway to get Firefox OS working on devices that only have 128 MiB of RAM. The codename for these devices is Tarako (https://wiki.mozilla.org/FirefoxOS/Tarako). In case it's not obvious, the memory situation on these devices is *tight*. Optimizations that wouldn't have been worthwhile in the desktop-only days are now worthwhile. For example, an optimization that saves 100 KiB of memory per process is pretty worthwhile for Firefox OS. On a phone, memory consumption can be the difference between something working and something not working in a much more clear-cut way than is typical on desktop. Conversely, landing new features that use extra memory need to be considered carefully. If you're planning a new feature and it will only use a few MiB on Firefox OS, think again. We probably cannot afford it. https://bugzilla.mozilla.org/show_bug.cgi?id=975367 is the bug that motivated me to write this email, but I'm also reminded of Firefox 4, which was a release with very high memory consumption partly because several groups independently made decisions that using extra memory was ok for a particular sub-system and the combined effect was bad. We live in a memory-constrained world. Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: We live in a memory-constrained world
On Mon, Feb 24, 2014 at 6:50 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: Nick, do we run any kind of automated leak checking through valgrind? I found bug 976363 today, it's something that any automated leak detection tool should be able to catch. The Valgrind test job does leak checking, and it's recently caught some leaks and caused the offending patches to be backed out. However, the test coverage is pretty meagre, and it's desktop only so can't detect leaks in IPC code. I believe the ASan builds also do leak checking. Their coverage is much better, but still doesn't cover IPC. Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: We live in a memory-constrained world
- Original Message - The Valgrind test job does leak checking, and it's recently caught some leaks and caused the offending patches to be backed out. However, the test coverage is pretty meagre, and it's desktop only so can't detect leaks in IPC code. I believe the ASan builds also do leak checking. Their coverage is much better, but still doesn't cover IPC. I don't think we run any leak checking with ASan, or at least I've never seen such a report. I filed bug 976414 for investigating LeakSanitizer. Andrew ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Proposed W3C Charter: Timed Text Working Group (WebVTT and TTML)
The W3C is proposing a revised charter for: Timed Text Working Group http://lists.w3.org/Archives/Public/public-new-work/2014Feb/0004.html http://www.w3.org/2013/10/timed-text-charter.html deadline for comments: March 20 This new charter is quite substantive, in that it recharters a working group that was previously only for TTML to now be to develop both TTML and WebVTT. My understanding is that the two halves of the group are expected to operate somewhat separately but also interact, although the charter doesn't seem to say that explicitly. Mozilla has the opportunity to send comments or objections through March 20. Please reply to this thread if you think there's something we should say. -David -- 턞 L. David Baron http://dbaron.org/ 턂 턢 Mozilla http://www.mozilla.org/ 턂 signature.asc Description: Digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Proposed W3C Charter: Geolocation Working Group
The W3C is proposing a revised charter for: Geolocation Working Group http://lists.w3.org/Archives/Public/public-new-work/2014Feb/0002.html http://www.w3.org/2014/02/geo-charter.html deadline for comments: March 15 I don't know any of the background on this one. Mozilla has the opportunity to send comments or objections through March 15. Please reply to this thread if you think there's something we should say. -David -- 턞 L. David Baron http://dbaron.org/ 턂 턢 Mozilla http://www.mozilla.org/ 턂 signature.asc Description: Digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
