Re: Hiding 'new' statements - Good or Evil?
I'm not sure what the benefits are (the MakeUnique comments only really seem to give aesthetic ones), but they do make it a bit harder when searching through code to see where things are constructed. I suppose if we always stick to using Make* then (with regex) it wouldn't add too much burden for searching. On Thursday, 23 November 2017 07:50:27 UTC, gsqu...@mozilla.com wrote: > Should we allow hiding 'new' statements, or keep them as visible as possible? > > > Some context: > Recently in bug 1410252 I added a MakeNotNull(args...) function that does > `NotNull(new T(args...))`, in the style of MakeUnique and others. It also > works with RefPtr. > > My first goal was to avoid the unnecessary nullptr-check in the NotNull > constructor, since our new is infallible by default. > > And I thought that hiding the naked new statement was a nice side benefit, as > I was under the impression that it was A Good Thing in modern C++. (Though I > think the main reason for that, was to prevent leaks when handling exceptions > in multi-allocation expressions, so it doesn't apply to us here.) > > Now, a colleague remarked that this was making the memory allocation less > obvious. > "What about MakeUnique?" I asked; "I'm used to them now" he retorted. :-P > > > So, what do you all think? > - Should I remove MakeNotNull? > - Or should we allow/encourage more MakeX functions instead of X(new...)? I'm > thinking MakeRefPtr might be nice. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Hiding 'new' statements - Good or Evil?
I agree that MakeNotNull doesn't sound like it allocates. Reads to me as Make[Into]NotNull. Context will *usually* make this clear, but why not NewNotNull? (Honestly I don't like MakeUnique to begin with) NotNull::New(args...) is another option. "My first goal was to avoid the unnecessary nullptr-check in the NotNull constructor, since our new is infallible by default." Even if we can't communicate to the optimizer that new is infallible and incur it to omit the branch in this inlined code, it would be extremely well predicted. MOZ_UNLIKELY() it and call it good, IMO. On Thu, Nov 23, 2017 at 12:58 AM, wrote: > I'm not sure what the benefits are (the MakeUnique comments only really seem > to give aesthetic ones), but they do make it a bit harder when searching > through code to see where things are constructed. > > I suppose if we always stick to using Make* then (with regex) it wouldn't add > too much burden for searching. > > On Thursday, 23 November 2017 07:50:27 UTC, gsqu...@mozilla.com wrote: >> Should we allow hiding 'new' statements, or keep them as visible as possible? >> >> >> Some context: >> Recently in bug 1410252 I added a MakeNotNull(args...) function that >> does `NotNull(new T(args...))`, in the style of MakeUnique and others. >> It also works with RefPtr. >> >> My first goal was to avoid the unnecessary nullptr-check in the NotNull >> constructor, since our new is infallible by default. >> >> And I thought that hiding the naked new statement was a nice side benefit, >> as I was under the impression that it was A Good Thing in modern C++. >> (Though I think the main reason for that, was to prevent leaks when handling >> exceptions in multi-allocation expressions, so it doesn't apply to us here.) >> >> Now, a colleague remarked that this was making the memory allocation less >> obvious. >> "What about MakeUnique?" I asked; "I'm used to them now" he retorted. :-P >> >> >> So, what do you all think? >> - Should I remove MakeNotNull? >> - Or should we allow/encourage more MakeX functions instead of X(new...)? >> I'm thinking MakeRefPtr might be nice. > > ___ > 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: Hiding 'new' statements - Good or Evil?
On 23/11/2017 12:05, Jeff Gilbert wrote: I agree that MakeNotNull doesn't sound like it allocates. Reads to me as Make[Into]NotNull. Context will *usually* make this clear, but why not NewNotNull? (Honestly I don't like MakeUnique to begin with) As far as naming is concerned, ISTM that "Create..." would be clearer than "Make..." for methods like this, as it's not so easy to misinterpret as "make [an existing thing] into [a certain state]". JK ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Firefox 55.* in Windows/Ubuntu - every day CPU 100%/Hangs Up. Please to do something!
And fresh reports too from today: Linux Mitn (may be without addons too): https://crash-stats.mozilla. com/report/index/a512c1fa-f6c1-4c89-8649-489c71171121 Ubuntu - during i am writting this email (!): https://crash-stats.mozilla.com/report/index/b25bb2ba-45dd-4124-bc9c-ad8ef1171121 Ubuntu - other today CPU eating: https://crash-stats.mozilla.com/report/index/a67f3d6e-b0e0-4069-9908-a462b1171121 I hope it wll help to you to catch anoying bug... 2017-11-22 0:19 GMT+01:00 Alexey Zvyagin : > Hello All, > > Today i had set up new computer (Linux Mint) > Before finish i set up a syncing. But before i catch permanent CPU eating > in new Firefox i saw that there no addons. Only bookmarked were synced. > After restarting and posting crash report i checked Firefox - there were > not addons yet. So i think it's crash report as in Safe mode. > So i think this trouble without addons, only during syncing. > > Crash report (Linux Mint 18.02 64bit, without addons). During crash i > pressed Bookmrk about previous crash and had beed filling tags of > bookmark... > > https://crash-stats.mozilla.com/report/index/f4085463-a792- > 4101-ad3f-3e36d1171121 > > P.S. When i have been writting this email my Firefox hanged up agian > (Ubuntu) > Crash report: https://crash-stats.mozilla.com/report/index/cbbb17c1-c3f0- > 454c-ba8b-22a381171121 > > 2017-11-21 21:32 GMT+01:00 Kris Maglione : > >> On Tue, Nov 21, 2017 at 02:22:26PM -0500, Ted Mielczarek wrote: >> >>> On Tue, Nov 21, 2017, at 06:54 AM, Alexey Zvyagin wrote: >>> Hi! I made some crashes by hands (crashfirefox.exe) in Windows 7 and in Unix through kill -ABRT What are the symptoms? In random moments the Firefox v56.* has only-one core CPU 100% eating. In Windows 7 (64bit) & Linux (Ubuntu 16.04 LTE 64bit) Reports are here: Ubuntu OS: https://crash-stats.mozilla.com/report/index/0b0e6273-26fb-4 82e-b033-c91be1171101 https://crash-stats.mozilla.com/report/index/237ae0e4-6eb2-4 c8b-87e8-3c2471171101 https://crash-stats.mozilla.com/report/index/7ddfad60-8f3e-4 495-a05f-5d6d21171110 https://crash-stats.mozilla.com/report/index/95468eb1-28b2-4 0f7-8f0c-8a7261171110 https://crash-stats.mozilla.com/report/index/cd310102-c547-4 86f-bbd3-0b7791171110 https://crash-stats.mozilla.com/report/index/6df179b4-721a-4 440-97e5-059d21171110 Windows 7: https://crash-stats.mozilla.com/report/index/8e172c11-2367-4 3b1-98f2-128251171113#allthreads >>> >>> Hi Alexey, >>> >>> These crashes all seem to be stuck in sqlite code querying the places >>> database that contains browsing history. In the Windows crash, you can >>> see that thread 0 is waiting on a sqlite mutex for the database, and >>> thread 44 is in the middle of running some sort of sqlite query, so >>> presumably it's holding the mutex. >>> >> >> These are actually all bookmark queries, and unfortunately, main-thread, >> blocking bookmark queries, at that. >> >> My best guess would also be an extension, but WebExtensions at least >> should not be hitting these code paths. >> > > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Firefox 55.* in Windows/Ubuntu - every day CPU 100%/Hangs Up. Please to do something!
Hello All, Today i had set up new computer (Linux Mint) Before finish i set up a syncing. But before i catch permanent CPU eating in new Firefox i saw that there no addons. Only bookmarked were synced. After restarting and posting crash report i checked Firefox - there were not addons yet. So i think it's crash report as in Safe mode. So i think this trouble without addons, only during syncing. Crash report (Linux Mint 18.02 64bit, without addons). During crash i pressed Bookmrk about previous crash and had beed filling tags of bookmark... https://crash-stats.mozilla.com/report/index/f4085463- a792-4101-ad3f-3e36d1171121 P.S. When i have been writting this email my Firefox hanged up agian (Ubuntu) Crash report: https://crash-stats.mozilla.com/report/index/cbbb17c1- c3f0-454c-ba8b-22a381171121 2017-11-21 21:32 GMT+01:00 Kris Maglione : > On Tue, Nov 21, 2017 at 02:22:26PM -0500, Ted Mielczarek wrote: > >> On Tue, Nov 21, 2017, at 06:54 AM, Alexey Zvyagin wrote: >> >>> Hi! >>> >>> I made some crashes by hands (crashfirefox.exe) in Windows 7 and in Unix >>> through kill -ABRT >>> >>> What are the symptoms? In random moments the Firefox v56.* has only-one >>> core CPU 100% eating. In Windows 7 (64bit) & Linux (Ubuntu 16.04 LTE >>> 64bit) >>> >>> Reports are here: >>> >>> Ubuntu OS: >>> >>> https://crash-stats.mozilla.com/report/index/0b0e6273-26fb- >>> 482e-b033-c91be1171101 >>> https://crash-stats.mozilla.com/report/index/237ae0e4-6eb2- >>> 4c8b-87e8-3c2471171101 >>> https://crash-stats.mozilla.com/report/index/7ddfad60-8f3e- >>> 4495-a05f-5d6d21171110 >>> https://crash-stats.mozilla.com/report/index/95468eb1-28b2- >>> 40f7-8f0c-8a7261171110 >>> https://crash-stats.mozilla.com/report/index/cd310102-c547- >>> 486f-bbd3-0b7791171110 >>> https://crash-stats.mozilla.com/report/index/6df179b4-721a- >>> 4440-97e5-059d21171110 >>> >>> Windows 7: >>> >>> https://crash-stats.mozilla.com/report/index/8e172c11-2367- >>> 43b1-98f2-128251171113#allthreads >>> >> >> Hi Alexey, >> >> These crashes all seem to be stuck in sqlite code querying the places >> database that contains browsing history. In the Windows crash, you can >> see that thread 0 is waiting on a sqlite mutex for the database, and >> thread 44 is in the middle of running some sort of sqlite query, so >> presumably it's holding the mutex. >> > > These are actually all bookmark queries, and unfortunately, main-thread, > blocking bookmark queries, at that. > > My best guess would also be an extension, but WebExtensions at least > should not be hitting these code paths. > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Firefox 55.* in Windows/Ubuntu - every day CPU 100%/Hangs Up. Please to do something!
Linux Mint - it's new computer. I started there syncing only today. I have Firefox in work - there are Windows 7 and same problem... (i sent crash reports from Windows) I don't think that there in all computers are corrupted DB... I turned off now all bookmark related add-ons: "Go parent folder", "Open Bookmarks in New Tab", "Show Parent Folder" I will research this problem and will let to know about result > Please file a bug for this issue, and we can take the discussion there. This list isn't really the right place for it. Ok. I will do tomorrow. Thanks! Bye! 2017-11-22 0:27 GMT+01:00 Marco Bonardo : > Is your profile local, or do you have it on a network share? > > It's possible your db has some kind of corruption that causes > recursive references. > I'd first of all check it using sqlite3 and the PRAGMA integrity_check > query. > If that passes (returns "ok"), I'd also try to run the Places > Maintenance from about:support. > > As previously said, could also be an extension, since you are on 56 > you are using legacy extensions, that can do whatever they wish with > our APIs, including some of the slow paths we removed in 57 and 58 > beta. > > Please let me know if you have any news, also, tracking this in a bug > could be useful, regardless of the final outcome. > > > On Tue, Nov 21, 2017 at 9:32 PM, Kris Maglione > wrote: > > On Tue, Nov 21, 2017 at 02:22:26PM -0500, Ted Mielczarek wrote: > >> > >> On Tue, Nov 21, 2017, at 06:54 AM, Alexey Zvyagin wrote: > >>> > >>> Hi! > >>> > >>> I made some crashes by hands (crashfirefox.exe) in Windows 7 and in > Unix > >>> through kill -ABRT > >>> > >>> What are the symptoms? In random moments the Firefox v56.* has only-one > >>> core CPU 100% eating. In Windows 7 (64bit) & Linux (Ubuntu 16.04 LTE > >>> 64bit) > >>> > >>> Reports are here: > >>> > >>> Ubuntu OS: > >>> > >>> > >>> https://crash-stats.mozilla.com/report/index/0b0e6273- > 26fb-482e-b033-c91be1171101 > >>> > >>> https://crash-stats.mozilla.com/report/index/237ae0e4- > 6eb2-4c8b-87e8-3c2471171101 > >>> > >>> https://crash-stats.mozilla.com/report/index/7ddfad60- > 8f3e-4495-a05f-5d6d21171110 > >>> > >>> https://crash-stats.mozilla.com/report/index/95468eb1- > 28b2-40f7-8f0c-8a7261171110 > >>> > >>> https://crash-stats.mozilla.com/report/index/cd310102- > c547-486f-bbd3-0b7791171110 > >>> > >>> https://crash-stats.mozilla.com/report/index/6df179b4- > 721a-4440-97e5-059d21171110 > >>> > >>> Windows 7: > >>> > >>> > >>> https://crash-stats.mozilla.com/report/index/8e172c11- > 2367-43b1-98f2-128251171113#allthreads > >> > >> > >> Hi Alexey, > >> > >> These crashes all seem to be stuck in sqlite code querying the places > >> database that contains browsing history. In the Windows crash, you can > >> see that thread 0 is waiting on a sqlite mutex for the database, and > >> thread 44 is in the middle of running some sort of sqlite query, so > >> presumably it's holding the mutex. > > > > > > These are actually all bookmark queries, and unfortunately, main-thread, > > blocking bookmark queries, at that. > > > > My best guess would also be an extension, but WebExtensions at least > should > > not be hitting these code paths. > > > > ___ > > 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: Hiding 'new' statements - Good or Evil?
I think it makes sense to hide a 'new' call in a Make* function when you're writing an abstraction that handles allocation *and* deallocation. So MakeUnique makes sense, because UniquePtr takes ownership of the allocated object, and will deallocate it in the destructor. MakeRefPtr would make sense for the same reason. But in cases where a library facility is not taking ownership of the object, and thus the user will need to write an explicit 'delete', it makes sense to require that the user also write an explicit 'new', for symmetry. NotNull is a bit of a weird case because it can wrap either a raw pointer or a smart pointer, so it doesn't clearly fit into either category. Perhaps it would make sense for MakeNotNull to only be usable with smart pointers? Cheers, Botond ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Hiding 'new' statements - Good or Evil?
On 11/23/2017 11:54 PM, Botond Ballo wrote: I think it makes sense to hide a 'new' call in a Make* function when you're writing an abstraction that handles allocation *and* deallocation. So MakeUnique makes sense, because UniquePtr takes ownership of the allocated object, and will deallocate it in the destructor. MakeRefPtr would make sense for the same reason. I almost agree with this, but, all these Make* variants hide the information that they are allocating, and since allocation is slow, it is usually better to know when allocation happens. I guess I'd prefer UniquePtr::New() over MakeUnique to be more clear about the functionality. -Olli But in cases where a library facility is not taking ownership of the object, and thus the user will need to write an explicit 'delete', it makes sense to require that the user also write an explicit 'new', for symmetry. NotNull is a bit of a weird case because it can wrap either a raw pointer or a smart pointer, so it doesn't clearly fit into either category. Perhaps it would make sense for MakeNotNull to only be usable with smart pointers? Cheers, Botond ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
