Enabling |fdatasync| for Thunderbird compilation under linux
Hi, This was discussed before somewhere (in bugzilla ?) although I could not find it now) I noticed a rather slow processing of POP3 e-mail fetching on a PC with scsi disk. At each message download, I can hear a clicking sound coming from a SCSI disk suggesting a large amount of disk activity. Well, I can certainly understand that TB probably uses |fdatasync()| or |fsync()| to make sure the downloaded message reaches the disk. But I was curious since the sound I hear is a little too much for a single message download. I began wondering that maybe TB was using |fsync()| where |fdatasync()| was enough, and that may have slowed down download process unduly. Last time,I raised the issue, somehow I could not find the reference to |fsync| in my local comm-central source tree, and maybe I was not searching it correctly. Yesterday, I noticed that, using mxr.mozilla.org, that |fdatasync| is defined into |fsync| if it is not defined at compile time in sqlite3.c: From mxr.mozilla.org: mozilla/db/sqlite3/src/sqlite3.c (View Hg log or Hg annotations) line 27053 -- #if !defined(fdatasync) mozilla/security/nss/lib/sqlite/sqlite3.c (View Hg log or Hg annotations) line 26107 -- #if !defined(fdatasync) mozilla/toolkit/components/osfile/modules/osfile_unix_back.jsm (View Hg log or Hg annotations) line 392 -- libc.declareLazyFFI(SysFile, fdatasync, line 393 -- fdatasync, ctypes.default_abi, Quoting the first entry: 27047 /* 27048 ** We do not trust systems to provide a working fdatasync(). Some do. 27049 ** Others do no. To be safe, we will stick with the (slightly slower) 27050 ** fsync(). If you know that your system does support fdatasync() correctly, 27051 ** then simply compile with -Dfdatasync=fdatasync 27052 */ 27053 #if !defined(fdatasync) 27054 # define fdatasync fsync 27055 #endif Fair enough. I use linux and this is 2014. fdatasync() ought to work. So I re-compiled C-C thunderbird after adding -Dfdatasync=fdatasync to CC and CXX macro and (re-)configure. And I did some benchmarking of download time: Test method: I sent 2K short e-mails (1024 x is 2+ KB, and 1024 x 5+ KB ) messages to a local test account, and measured the download time from the local dovecot pop3 server to this account by clicking the download message button of TB explicitly (TB was configured not to download messages automatically). I used a stop watch style program to measure the time and so I only give the second resolution data below. Download time of 2024 e-mails: Before the change (using fsync() in sqlite3) 39 sec. 22 sec. 33 sec 24 sec 32 sec (note the wide variation.) After the change (using -Dfdatasync=fdatasync) 25 sec 21 sec 23 sec 21 sec 22 sec (note the narrower variation.) It is kind of difficult to say that using |fdatasync| shortened the download time. This is because the time with |fsync| varies too much. Then I realized that the wide variation of execution time is exactly because TB was using |fsync()|. Even on my local machine, there are many daemons that write data to log files, and creating and sending 2K e-mails with the following command also cause I/O traffic between the tests. for i in $(seq 1 1024); do echo $i; /usr/bin/mail -s test $i mtest2 /etc/rygel.conf ; /usr/bin/mail -s test 2 - $i mtest2 /etc/rsyslog.conf ; done cf. /etc/rygel.conf is 5078 bytes long, and /etc/rsyslog.conf is 2632 bytes long on my PC So depending on the background I/O activity, blindly using |fsync()| instead of |fdatasync()| causes too much elapsed time variation. This is indeed felt as undue slowdown sometimes for real. More to the point. I repeat the above test in the following manner: Initial step. I leave TB window open. Repeat: Send 2K messages as above to a local account. - download 2K messages (and measure the time.) into Inbox folder - delete the downloaded 2K messages (basically they are moved from Inbox to Trash folder: I can see the movement by the progress counter at the lower-left corner of the window pane.) - compat Inbox folder - empty Trash folder If necessary, repeat above. Now with |fsync()| the deletion of 2K messages (moving them from Inbox to Trash) is very sluggish and again its timing is widely varied. (I should have measured THAT time). OTOH, with |fdatasync| the time to move 2K messages is steady and constant. Obviously, unnecessary use of |fsync| is causing sometimes unnecessary delay while the movement of messages continue. The deletion of 2K messages is much nicer experience with |fdatasync| since the counter increments steadily without any strange pause that was often noticed with |fsync| and I definitely think the speed is faster with |fdatasync|. With downloading, I think the overhead of moving data via network interface (even with on local PC) is too much and masks the advantage of |fdatasync|. But with local DISK COPY of messages from one folder to the other (during deletion), the advantage of |fdatasync| is obvious. I tested TB
Re: Intent to implement and ship: navigator.hardwareConcurrency
I think it might help your case to acknowledge the often significant difference between technically possible, but expensive and unreliable and extremely simple and 100% reliable. That something is already technically possible does not mean that making it easier has no consequences. Arguing that the incremental fingerprinting risk is negligible is reasonable, but you lose credibility if you suggest it doesn't exist. Gavin On Tue, May 20, 2014 at 12:30 AM, Rik Cabanier caban...@gmail.com wrote: On Tue, May 20, 2014 at 12:24 AM, Jonas Sicking jo...@sicking.cc wrote: On Mon, May 19, 2014 at 7:14 PM, Mike Hommey m...@glandium.org wrote: On Mon, May 19, 2014 at 06:35:49PM -0700, Jonas Sicking wrote: On Mon, May 19, 2014 at 4:10 PM, Rik Cabanier caban...@gmail.com wrote: I don't see why the web platform is special here and we should trust that authors can do the right thing. I'm fairly sure people have already pointed this out to you. But the reason the web platform is different is that because we allow arbitrary application logic to run on the user's device without any user opt-in. I.e. the web is designed such that it is safe for a user to go to any website without having to consider the risks of doing so. This is why we for example don't allow websites to have arbitrary read/write access to the user's filesystem. Something that all the other platforms that you have pointed out do. Those platforms instead rely on that users make a security decision before allowing any code to run. This has both advantages (easier to design APIs for those platforms) and disadvantages (malware is pretty prevalent on for example Windows). As much as I agree the API is not useful, I don't buy this argument either. What prevents a web app to just use n workers, where n is a much bigger number than what would be returned by the API? Nothing. The attack I'm trying to prevent is fingerprinting. Allowing workers to run a large number of workers does not allow fingerprinting. Eli's polyfill can already be used to do fingerprinting [1]. It's not very good at giving a consistent and accurate results which makes it less suitable to plan your workload. It also wastes a lot of CPU cycles. 1: http://wg.oftn.org/projects/core-estimator/demo/ ___ 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
using namespace
Now that we have unified builds, writing using namespace in the global scope of a .cpp file is almost as bad as writing it in a header. Regularly build errors show up like this one: https://tbpl.mozilla.org/php/getParsedLog.php?id=40010766tree=Try What's awesome about these errors is that they typically show up on some platform but not the others and they come and go as we add files or do things that can alter the generation of the unified .cpp files. So, I strongly encourage everyone to stop using using namespace. If you really really want to use them, please put them inside the scope of a namespace. For instance a .cpp file in the gfx/layers directory typically looks like this: // using namespace gfx; here means nical: r- namespace mozilla { namespace layers { // using namespace gfx; here means nical is grumpy but he'll probably not block your super important path for this. // ... } } Beware that once using namespace is placed in an enclosing namespace foo, it will affect the namespace foo the next time you open it: namespace foo { using namespace unicorns; // ... } namespace foo { // here it's just like you wrote using namespace unicorns; again } Which leads to problems with namespaces like mozilla::ipc and mozilla::dom::ipc being ambiguous in the mozilla namespace. it is probably just not possible to write using namespace mozilla::dom; in most of the layers code for this reason. Honestly, I don't mean to start a bikeshed about whether we should enforce strict rules about this project-wise, because I know that people have different tastes as to whether writing Size is vastly better than gfx::Size. But I'd like to encourage people to carefully make use of this c++ feature, especially when the namespace in question is used a handful of times in the file. I personally will be strict about it the reviews I give. Cheers, Nical ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to ship: Hyperlink Auditing (a ping)
On Fri 16 May 2014 07:45:22 AM PDT, Justin Dolske wrote: On 5/16/14, 6:38 AM, Curtis Koenig wrote: Would this be disabled in Private Browsing? If not that might be perceived as negating one of the reasons users have for using that particular feature. Private Browsing mode is about not storing _local_ data from your activities. It is explicitly not an anti tracking mode because that's extremely difficult-to-impossible to do robustly just on the client, and would be a misleading claim and/or result in a browser most people would think is broken. E.G. as already noted in this thread, sites can already do this without a ping. When you're shopping for an engagement ring, why would you want to prevent the jewelry vendor from knowing that you're shopping, or from knowing who you are? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: using namespace
(2014/05/21 8:51), Robert O'Callahan wrote: Personally I find unified-build-related using-namespace errors are rare --- I've not encountered one yet. I'm not sure they're worth attempting to ameliorate. I wasted half a day recently due to a using namespace mozilla::layers in an unrelated file (/layout/style/AnimationCommon.cpp) causing clashes. (I replaced it with using mozilla::layers::Layer which I guess still has problems.) Brian ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to ship: Hyperlink Auditing (a ping)
On 2014-05-20, 6:43 PM, Jonas Sicking wrote: On Tue, May 20, 2014 at 2:33 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: But I believe that that would be a pretty crappy private browsing feature which I don't think anyone here would argue for. Private browsing is mainly about giving you a new, throw-away, identity. The throw-away part is why we don't allow storing data. The reason we have a separate cookie jar is in order to implement the new part. That was actually an unintended use case which was enabled as a side-effect of the cookie jar separation. We never really designed PB for this. Whatever the reason we did this originally was, I believe that we would have had a mostly useless private-browsing feature if we had not created a new blank cookie-jar for private browsing. I really do believe that private browsing must create a new, throw-away profile. Anything else will be mostly useless to users. We're diverging from the topic of this thread, but to me, that's an implementation detail as long as we ensure that the users' activity in PB mode doesn't leak into their normal session. They don't care if google didn't technically create any new cookies, if wedding ring shows up in the user's search history due to a search done in private browsing, they will be very disappointed. That's true, and it's the reason why we made this choice for handling cookies. As long as we ensure that the channel opened to deliver the ping from a ping inherits the correct privacy settings, it should work fine with the rest of the stack, ensuring that your non-private cookies are not sent alongside the ping, the ping doesn't get cached to disk, etc. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: using namespace
On 2014-05-20, 9:11 PM, L. David Baron wrote: On Wednesday 2014-05-21 11:51 +1200, Robert O'Callahan wrote: On Wed, May 21, 2014 at 4:36 AM, Nicolas Silva nical.si...@gmail.comwrote: Honestly, I don't mean to start a bikeshed about whether we should enforce strict rules about this project-wise, because I know that people have different tastes as to whether writing Size is vastly better than gfx::Size. But I'd like to encourage people to carefully make use of this c++ feature, especially when the namespace in question is used a handful of times in the file. I personally will be strict about it the reviews I give. I don't think we should have rules that some reviewers enforce and others don't. That makes life hard for everyone. It's not clear to me how moving the using namespace declaration inside a namespace helps. Won't most files in a unified build translation unit put their declarations in the same namespace, negating any benefit? Personally I find unified-build-related using-namespace errors are rare --- I've not encountered one yet. I'm not sure they're worth attempting to ameliorate. I wonder if the problem is that we're overusing namespaces (i.e., we have too many of them or put many classes at places too deeply nested in the namespace hierarchy)? Perhaps it makes sense to have a guideline that names shouldn't feel overly verbose with only a using namespace mozilla? For example, that would suggest that layers::Layer feeling redundant is a sign that Layer should instead be directly in the mozilla namespace. I think following such a guideline (and avoiding using declarations of the inner namespaces) might help avoiding the various collision problems we've hit. The problem is that we're using nested namespaces with same names under them. C++ doesn't really provide a good mechanism to fix this problem, which means we have to create arbitrary restrictions on things such as where using namespaces go. FWIW, I argued against nested namespaces a few years ago (couldn't find a link to it through Google unfortunately) and people let me win that battle by allowing me to edit the coding style to prohibit nested namespoaces in most cases https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Namespaces, but unfortunately we never adhered to this rule in practice, and these days three level nested namespaces are pretty common in the code base. We're just bending C++ in a way that it's not quite comfortable with here. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: using namespace
On Wed, May 21, 2014 at 1:11 PM, L. David Baron dba...@dbaron.org wrote: I wonder if the problem is that we're overusing namespaces (i.e., we have too many of them or put many classes at places too deeply nested in the namespace hierarchy)? Perhaps it makes sense to have a guideline that names shouldn't feel overly verbose with only a using namespace mozilla? As Ehsan says, we already have that guideline. Rob -- Jtehsauts tshaei dS,o n Wohfy Mdaon yhoaus eanuttehrotraiitny eovni le atrhtohu gthot sf oirng iyvoeu rs ihnesa.rt sS?o Whhei csha iids teoa stiheer :p atroa lsyazye,d 'mYaonu,r sGients uapr,e tfaokreg iyvoeunr, 'm aotr atnod sgaoy ,h o'mGee.t uTph eann dt hwea lmka'n? gBoutt uIp waanndt wyeonut thoo mken.o w ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Do we still need Trace Malloc?
On 2014-05-19, 10:25 AM, Nicholas Nethercote wrote: It's used to get stacks within the deadlock detector, but I'm not sure if that's necessary, and it doesn't seem like it would be that hard to replace if it is necessary. You would think so, but I tried in bug 939231 and failed. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: using namespace
On 5/20/2014 8:37 PM, Ehsan Akhgari wrote: FWIW, I argued against nested namespaces a few years ago (couldn't find a link to it through Google unfortunately) and people let me win that battle by allowing me to edit the coding style to prohibit nested namespoaces in most cases https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Namespaces, but unfortunately we never adhered to this rule in practice, and these days three level nested namespaces are pretty common in the code base. We're just bending C++ in a way that it's not quite comfortable with here. How about adding the rule all new namespaces must be approved by insert specific top-level superreviewer here, just like all new top-level directories need to be explicitly approved? If we could get reviewers to enforce that, it would hopefully cut down on people using outlandishly long namespaces. I think there are valid reasons to have a two-level namespace (e.g., mozilla::mailnews [1]), but I find it deathly hard to justify going any deeper. [1] Actually, I would probably prefer mozilla::comm had mozilla::mailnews not already had precedent when I started using C++ namespaces. :-) -- Joshua Cranmer Thunderbird and DXR developer Source code archæologist ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: using namespace
On 2014-05-20, 10:00 PM, Joshua Cranmer wrote: On 5/20/2014 8:37 PM, Ehsan Akhgari wrote: FWIW, I argued against nested namespaces a few years ago (couldn't find a link to it through Google unfortunately) and people let me win that battle by allowing me to edit the coding style to prohibit nested namespoaces in most cases https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Namespaces, but unfortunately we never adhered to this rule in practice, and these days three level nested namespaces are pretty common in the code base. We're just bending C++ in a way that it's not quite comfortable with here. How about adding the rule all new namespaces must be approved by insert specific top-level superreviewer here, just like all new top-level directories need to be explicitly approved? If we could get reviewers to enforce that, it would hopefully cut down on people using outlandishly long namespaces. I don't think that will work. IIRC I approached some module owners about this back in the day and they explicitly told me that they don't care and asked me to fix the rest of the places where we use nested namespaces, and I ended up giving up on the whole idea. I think there are valid reasons to have a two-level namespace (e.g., mozilla::mailnews [1]), but I find it deathly hard to justify going any deeper. [1] Actually, I would probably prefer mozilla::comm had mozilla::mailnews not already had precedent when I started using C++ namespaces. :-) See, this is the problem! Everyone thinks that it's reasonable to add their own nested namespaces, and they're right as long as they don't look at the big picture. As long as we allow any code to #include any other code, this is a slippery slope. Honestly, speaking as the person who did the majority of the work to give us unified builds, and presumably dealt with the most of this issue while doing so, this is not a very important problem, and it's uncommon enough that I don't think it warrants an effort to get a handle on our namespace usage or add stricter review requirements. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform