Re: Memory leak from list
On Sun, Feb 23, 2020 at 05:00:43PM -0500, Richard Kimberly Heck wrote: > On 2/23/20 4:11 PM, Scott Kostyshak wrote: > > On Sun, Feb 23, 2020 at 03:54:06PM -0500, Richard Kimberly Heck wrote: > >> On 2/23/20 2:31 PM, Scott Kostyshak wrote: > >>> On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote: > On 2/23/20 8:23 AM, Scott Kostyshak wrote: > > On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote: > >> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: > >>> On 2/18/20 6:07 PM, Scott Kostyshak wrote: > Valgrind gave me the following error: > > ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are > definitely lost in loss record 5,165 of 5,862 > ==732==at 0x483AE63: operator new(unsigned long) (in > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const > (Buffer.cpp:661) > ==732==by 0x11E583C: lyx::(anonymous > namespace)::copyToTempBuffer(lyx::ParagraphList const&, > std::shared_ptr) (CutAndPaste.cpp:582) > ==732==by 0x11E5C6A: lyx::(anonymous > namespace)::putClipboard(lyx::ParagraphList const&, > std::shared_ptr, > std::__cxx11::basic_string, > std::allocator > const&, lyx::BufferParams) > (CutAndPaste.cpp:613) > ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor > const&, std::__cxx11::basic_string std::char_traits, std::allocator > const&) > (CutAndPaste.cpp:1123) > ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor > const&) (CutAndPaste.cpp:1024) > ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, > lyx::FuncRequest&) (Text3.cpp:1593) > ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, > lyx::FuncRequest&) (InsetText.cpp:339) > ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, > lyx::FuncRequest&) (Inset.cpp:325) > ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest > const&) (Cursor.cpp:825) > ==732==by 0x1816F4F: > lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest > const&, lyx::DispatchResult&) (GuiView.cpp:3878) > ==732==by 0x181B959: > lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, > lyx::DispatchResult&) (GuiView.cpp:4569) > > It comes from the following line (Buffer.cpp:661): > > cloned_buffers.push_back(new CloneList); > > Currently cloned_buffers is a list. Would it make sense > to make it a list of *smart* pointers instead? Alternatively we > could make a class and then make a custom destructor that would free > the CloneLists that the list elements point to? > >>> This is some kind of thinko, probably on my part. The code at line 549 > >>> was supposed to be cleaning this up, but it actually only removes the > >>> entry from the list. > >>> > >>> If it works to make it a smart pointer of some kind, then that would > >>> be > >>> simplest. But I think we could just do something like: > >>> > >>> else { > >>> delete(*it); > >>> cloned_buffers.erase(it); > >>> } > >> Ah that makes sense. > > Riki, I propose that you commit. Thanks for the fix. > Just to check: You've verified this fixes the problem? > >>> I just tried to reproduce the original error (without the fix) and could > >>> not. I originally got the message from copying something and compiling. > >>> I just tried doing that with the Customization and Embedded Objects > >>> manuals, but I could not trigger the error. > >> Committed, then. > > Thanks for the fix. I'll continue testing LyX with Valgrind at some > > point in the future. Right now I'm tired of the waiting. I might look > > into the suggestions that Neven gave. > > Actually, that is all wrong, and now I'm very confused. The patch causes > a double delete exception here, so I have reverted it. The line > > delete d->clone_list_; > > already does this deletion, since *it just is d->clone_list_. > > I've committed another patch with a shared_ptr. That has at least to help. OK thanks for your work on this. I'll keep an eye out when testing with Valgrind to see if something related comes up. Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/23/20 4:11 PM, Scott Kostyshak wrote: > On Sun, Feb 23, 2020 at 03:54:06PM -0500, Richard Kimberly Heck wrote: >> On 2/23/20 2:31 PM, Scott Kostyshak wrote: >>> On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote: On 2/23/20 8:23 AM, Scott Kostyshak wrote: > On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote: >> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: >>> On 2/18/20 6:07 PM, Scott Kostyshak wrote: Valgrind gave me the following error: ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely lost in loss record 5,165 of 5,862 ==732==at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const (Buffer.cpp:661) ==732==by 0x11E583C: lyx::(anonymous namespace)::copyToTempBuffer(lyx::ParagraphList const&, std::shared_ptr) (CutAndPaste.cpp:582) ==732==by 0x11E5C6A: lyx::(anonymous namespace)::putClipboard(lyx::ParagraphList const&, std::shared_ptr, std::__cxx11::basic_string, std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613) ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, std::__cxx11::basic_string, std::allocator > const&) (CutAndPaste.cpp:1123) ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) (CutAndPaste.cpp:1024) ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, lyx::FuncRequest&) (Text3.cpp:1593) ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, lyx::FuncRequest&) (InsetText.cpp:339) ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, lyx::FuncRequest&) (Inset.cpp:325) ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&) (Cursor.cpp:825) ==732==by 0x1816F4F: lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, lyx::DispatchResult&) (GuiView.cpp:3878) ==732==by 0x181B959: lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, lyx::DispatchResult&) (GuiView.cpp:4569) It comes from the following line (Buffer.cpp:661): cloned_buffers.push_back(new CloneList); Currently cloned_buffers is a list. Would it make sense to make it a list of *smart* pointers instead? Alternatively we could make a class and then make a custom destructor that would free the CloneLists that the list elements point to? >>> This is some kind of thinko, probably on my part. The code at line 549 >>> was supposed to be cleaning this up, but it actually only removes the >>> entry from the list. >>> >>> If it works to make it a smart pointer of some kind, then that would be >>> simplest. But I think we could just do something like: >>> >>> else { >>> delete(*it); >>> cloned_buffers.erase(it); >>> } >> Ah that makes sense. > Riki, I propose that you commit. Thanks for the fix. Just to check: You've verified this fixes the problem? >>> I just tried to reproduce the original error (without the fix) and could >>> not. I originally got the message from copying something and compiling. >>> I just tried doing that with the Customization and Embedded Objects >>> manuals, but I could not trigger the error. >> Committed, then. > Thanks for the fix. I'll continue testing LyX with Valgrind at some > point in the future. Right now I'm tired of the waiting. I might look > into the suggestions that Neven gave. Actually, that is all wrong, and now I'm very confused. The patch causes a double delete exception here, so I have reverted it. The line delete d->clone_list_; already does this deletion, since *it just is d->clone_list_. I've committed another patch with a shared_ptr. That has at least to help. Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Sun, Feb 23, 2020 at 03:54:06PM -0500, Richard Kimberly Heck wrote: > On 2/23/20 2:31 PM, Scott Kostyshak wrote: > > On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote: > >> On 2/23/20 8:23 AM, Scott Kostyshak wrote: > >>> On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote: > On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: > > On 2/18/20 6:07 PM, Scott Kostyshak wrote: > >> Valgrind gave me the following error: > >> > >> ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are > >> definitely lost in loss record 5,165 of 5,862 > >> ==732==at 0x483AE63: operator new(unsigned long) (in > >> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > >> ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const > >> (Buffer.cpp:661) > >> ==732==by 0x11E583C: lyx::(anonymous > >> namespace)::copyToTempBuffer(lyx::ParagraphList const&, > >> std::shared_ptr) (CutAndPaste.cpp:582) > >> ==732==by 0x11E5C6A: lyx::(anonymous > >> namespace)::putClipboard(lyx::ParagraphList const&, > >> std::shared_ptr, > >> std::__cxx11::basic_string, > >> std::allocator > const&, lyx::BufferParams) > >> (CutAndPaste.cpp:613) > >> ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, > >> std::__cxx11::basic_string, > >> std::allocator > const&) (CutAndPaste.cpp:1123) > >> ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) > >> (CutAndPaste.cpp:1024) > >> ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, > >> lyx::FuncRequest&) (Text3.cpp:1593) > >> ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, > >> lyx::FuncRequest&) (InsetText.cpp:339) > >> ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, > >> lyx::FuncRequest&) (Inset.cpp:325) > >> ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest > >> const&) (Cursor.cpp:825) > >> ==732==by 0x1816F4F: > >> lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, > >> lyx::DispatchResult&) (GuiView.cpp:3878) > >> ==732==by 0x181B959: > >> lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, > >> lyx::DispatchResult&) (GuiView.cpp:4569) > >> > >> It comes from the following line (Buffer.cpp:661): > >> > >> cloned_buffers.push_back(new CloneList); > >> > >> Currently cloned_buffers is a list. Would it make sense > >> to make it a list of *smart* pointers instead? Alternatively we could > >> make a class and then make a custom destructor that would free the > >> CloneLists that the list elements point to? > > This is some kind of thinko, probably on my part. The code at line 549 > > was supposed to be cleaning this up, but it actually only removes the > > entry from the list. > > > > If it works to make it a smart pointer of some kind, then that would be > > simplest. But I think we could just do something like: > > > > else { > > delete(*it); > > cloned_buffers.erase(it); > > } > Ah that makes sense. > >>> Riki, I propose that you commit. Thanks for the fix. > >> Just to check: You've verified this fixes the problem? > > I just tried to reproduce the original error (without the fix) and could > > not. I originally got the message from copying something and compiling. > > I just tried doing that with the Customization and Embedded Objects > > manuals, but I could not trigger the error. > > Committed, then. Thanks for the fix. I'll continue testing LyX with Valgrind at some point in the future. Right now I'm tired of the waiting. I might look into the suggestions that Neven gave. Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/23/20 2:31 PM, Scott Kostyshak wrote: > On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote: >> On 2/23/20 8:23 AM, Scott Kostyshak wrote: >>> On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote: On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: > On 2/18/20 6:07 PM, Scott Kostyshak wrote: >> Valgrind gave me the following error: >> >> ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely >> lost in loss record 5,165 of 5,862 >> ==732==at 0x483AE63: operator new(unsigned long) (in >> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) >> ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const >> (Buffer.cpp:661) >> ==732==by 0x11E583C: lyx::(anonymous >> namespace)::copyToTempBuffer(lyx::ParagraphList const&, >> std::shared_ptr) (CutAndPaste.cpp:582) >> ==732==by 0x11E5C6A: lyx::(anonymous >> namespace)::putClipboard(lyx::ParagraphList const&, >> std::shared_ptr, >> std::__cxx11::basic_string, >> std::allocator > const&, lyx::BufferParams) >> (CutAndPaste.cpp:613) >> ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, >> std::__cxx11::basic_string, >> std::allocator > const&) (CutAndPaste.cpp:1123) >> ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) >> (CutAndPaste.cpp:1024) >> ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, >> lyx::FuncRequest&) (Text3.cpp:1593) >> ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, >> lyx::FuncRequest&) (InsetText.cpp:339) >> ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, >> lyx::FuncRequest&) (Inset.cpp:325) >> ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest >> const&) (Cursor.cpp:825) >> ==732==by 0x1816F4F: >> lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, >> lyx::DispatchResult&) (GuiView.cpp:3878) >> ==732==by 0x181B959: >> lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, >> lyx::DispatchResult&) (GuiView.cpp:4569) >> >> It comes from the following line (Buffer.cpp:661): >> >> cloned_buffers.push_back(new CloneList); >> >> Currently cloned_buffers is a list. Would it make sense to >> make it a list of *smart* pointers instead? Alternatively we could make >> a class and then make a custom destructor that would free the CloneLists >> that the list elements point to? > This is some kind of thinko, probably on my part. The code at line 549 > was supposed to be cleaning this up, but it actually only removes the > entry from the list. > > If it works to make it a smart pointer of some kind, then that would be > simplest. But I think we could just do something like: > > else { > delete(*it); > cloned_buffers.erase(it); > } Ah that makes sense. >>> Riki, I propose that you commit. Thanks for the fix. >> Just to check: You've verified this fixes the problem? > I just tried to reproduce the original error (without the fix) and could > not. I originally got the message from copying something and compiling. > I just tried doing that with the Customization and Embedded Objects > manuals, but I could not trigger the error. Committed, then. Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Sun, Feb 23, 2020 at 12:50:42PM -0500, Richard Kimberly Heck wrote: > On 2/23/20 8:23 AM, Scott Kostyshak wrote: > > On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote: > >> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: > >>> On 2/18/20 6:07 PM, Scott Kostyshak wrote: > Valgrind gave me the following error: > > ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely > lost in loss record 5,165 of 5,862 > ==732==at 0x483AE63: operator new(unsigned long) (in > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const > (Buffer.cpp:661) > ==732==by 0x11E583C: lyx::(anonymous > namespace)::copyToTempBuffer(lyx::ParagraphList const&, > std::shared_ptr) (CutAndPaste.cpp:582) > ==732==by 0x11E5C6A: lyx::(anonymous > namespace)::putClipboard(lyx::ParagraphList const&, > std::shared_ptr, > std::__cxx11::basic_string, > std::allocator > const&, lyx::BufferParams) > (CutAndPaste.cpp:613) > ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, > std::__cxx11::basic_string, > std::allocator > const&) (CutAndPaste.cpp:1123) > ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) > (CutAndPaste.cpp:1024) > ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, > lyx::FuncRequest&) (Text3.cpp:1593) > ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, > lyx::FuncRequest&) (InsetText.cpp:339) > ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, > lyx::FuncRequest&) (Inset.cpp:325) > ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest > const&) (Cursor.cpp:825) > ==732==by 0x1816F4F: > lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, > lyx::DispatchResult&) (GuiView.cpp:3878) > ==732==by 0x181B959: > lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, > lyx::DispatchResult&) (GuiView.cpp:4569) > > It comes from the following line (Buffer.cpp:661): > > cloned_buffers.push_back(new CloneList); > > Currently cloned_buffers is a list. Would it make sense to > make it a list of *smart* pointers instead? Alternatively we could make > a class and then make a custom destructor that would free the CloneLists > that the list elements point to? > >>> This is some kind of thinko, probably on my part. The code at line 549 > >>> was supposed to be cleaning this up, but it actually only removes the > >>> entry from the list. > >>> > >>> If it works to make it a smart pointer of some kind, then that would be > >>> simplest. But I think we could just do something like: > >>> > >>> else { > >>> delete(*it); > >>> cloned_buffers.erase(it); > >>> } > >> Ah that makes sense. > > Riki, I propose that you commit. Thanks for the fix. > > Just to check: You've verified this fixes the problem? I just tried to reproduce the original error (without the fix) and could not. I originally got the message from copying something and compiling. I just tried doing that with the Customization and Embedded Objects manuals, but I could not trigger the error. Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/23/20 8:23 AM, Scott Kostyshak wrote: > On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote: >> On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: >>> On 2/18/20 6:07 PM, Scott Kostyshak wrote: Valgrind gave me the following error: ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely lost in loss record 5,165 of 5,862 ==732==at 0x483AE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const (Buffer.cpp:661) ==732==by 0x11E583C: lyx::(anonymous namespace)::copyToTempBuffer(lyx::ParagraphList const&, std::shared_ptr) (CutAndPaste.cpp:582) ==732==by 0x11E5C6A: lyx::(anonymous namespace)::putClipboard(lyx::ParagraphList const&, std::shared_ptr, std::__cxx11::basic_string, std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613) ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, std::__cxx11::basic_string, std::allocator > const&) (CutAndPaste.cpp:1123) ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) (CutAndPaste.cpp:1024) ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, lyx::FuncRequest&) (Text3.cpp:1593) ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, lyx::FuncRequest&) (InsetText.cpp:339) ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, lyx::FuncRequest&) (Inset.cpp:325) ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&) (Cursor.cpp:825) ==732==by 0x1816F4F: lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, lyx::DispatchResult&) (GuiView.cpp:3878) ==732==by 0x181B959: lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, lyx::DispatchResult&) (GuiView.cpp:4569) It comes from the following line (Buffer.cpp:661): cloned_buffers.push_back(new CloneList); Currently cloned_buffers is a list. Would it make sense to make it a list of *smart* pointers instead? Alternatively we could make a class and then make a custom destructor that would free the CloneLists that the list elements point to? >>> This is some kind of thinko, probably on my part. The code at line 549 >>> was supposed to be cleaning this up, but it actually only removes the >>> entry from the list. >>> >>> If it works to make it a smart pointer of some kind, then that would be >>> simplest. But I think we could just do something like: >>> >>> else { >>> delete(*it); >>> cloned_buffers.erase(it); >>> } >> Ah that makes sense. > Riki, I propose that you commit. Thanks for the fix. Just to check: You've verified this fixes the problem? Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Tue, Feb 18, 2020 at 08:28:33PM -0500, Scott Kostyshak wrote: > On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: > > On 2/18/20 6:07 PM, Scott Kostyshak wrote: > > > Valgrind gave me the following error: > > > > > > ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely > > > lost in loss record 5,165 of 5,862 > > > ==732==at 0x483AE63: operator new(unsigned long) (in > > > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > > > ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const > > > (Buffer.cpp:661) > > > ==732==by 0x11E583C: lyx::(anonymous > > > namespace)::copyToTempBuffer(lyx::ParagraphList const&, > > > std::shared_ptr) (CutAndPaste.cpp:582) > > > ==732==by 0x11E5C6A: lyx::(anonymous > > > namespace)::putClipboard(lyx::ParagraphList const&, > > > std::shared_ptr, > > > std::__cxx11::basic_string, > > > std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613) > > > ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, > > > std::__cxx11::basic_string, > > > std::allocator > const&) (CutAndPaste.cpp:1123) > > > ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) > > > (CutAndPaste.cpp:1024) > > > ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, > > > lyx::FuncRequest&) (Text3.cpp:1593) > > > ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, > > > lyx::FuncRequest&) (InsetText.cpp:339) > > > ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, > > > lyx::FuncRequest&) (Inset.cpp:325) > > > ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&) > > > (Cursor.cpp:825) > > > ==732==by 0x1816F4F: > > > lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, > > > lyx::DispatchResult&) (GuiView.cpp:3878) > > > ==732==by 0x181B959: > > > lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, > > > lyx::DispatchResult&) (GuiView.cpp:4569) > > > > > > It comes from the following line (Buffer.cpp:661): > > > > > > cloned_buffers.push_back(new CloneList); > > > > > > Currently cloned_buffers is a list. Would it make sense to > > > make it a list of *smart* pointers instead? Alternatively we could make a > > > class and then make a custom destructor that would free the CloneLists > > > that the list elements point to? > > > > This is some kind of thinko, probably on my part. The code at line 549 > > was supposed to be cleaning this up, but it actually only removes the > > entry from the list. > > > > If it works to make it a smart pointer of some kind, then that would be > > simplest. But I think we could just do something like: > > > > else { > > delete(*it); > > cloned_buffers.erase(it); > > } > > Ah that makes sense. Riki, I propose that you commit. Thanks for the fix. Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Wed, Feb 19, 2020 at 08:52:18AM +, Neven Sajko wrote: > > Well, actually the hardest part is waiting because LyX is very slow when > > run under valgrind. > > Try sanitizers instead. They are instrumentation that GCC or Clang can > include in executables. They do basically the same thing as Valgrind, > but should be much faster and since you are already compiling your own > Lyx and Qt, the sanitizers should be a more appropriate tool. There I do not always compile Qt. On my last attempt to compile Qt I came across an error and just used the pre-built libraries (and I think the corresponding -dbg packages). I did not initially realize that compiling Qt with the address sanitizers would make a big difference, but now I think I see the intuition: LyX might be responsible for a sanitizer error at the Qt level, and we would only catch that if we compile Qt with the address sanitizers. > are four sanitizers that you may want to compile with: asan, tsan, > msan, ubsan; each for a different class of bugs. > > Instructions: include -fsanitize=asan (replace asan with wanted > sanitizer if you want another one) to both compilation and linking > commands (GCC or Clang) of all libraries and code. It is also a good > idea to disable some compiler optimizations, in particular one should > use -fno-omit-frame-pointer. See > https://github.com/google/sanitizers/wiki for more information. > > By default the error reports will be output to stderr, but it is > possible to output them to a file by setting an environment variable > (search for log_path in > https://github.com/google/sanitizers/wiki/AddressSanitizerFlags if in > need of that option). > > I hope that was helpful. Thanks, Neven! This is helpful. I've been meaning to look into tools like this for a while, and this might be just the push I needed. Eventually I would like to add these options to the building and testing scripts at https://github.com/scottkosty/lyx-tester. It would be interesting to build with a sanitizer and then run our big suite of automated tests. I made an issue to remind me to look into this, although I probably won't look into it soon (just because of time reasons and other pending tasks): https://github.com/scottkosty/lyx-tester/issues/1 Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
My instructions for the C compiler and linker command line were wrong: instead of -fsanitize=asan , use -fsanitize=address or -fsanitize=thread or -fsanitize=undefined or -fsanitize=memory . And, of course, include debugging symbols with "-g". Regards, Neven Sajko -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
> Well, actually the hardest part is waiting because LyX is very slow when run > under valgrind. Try sanitizers instead. They are instrumentation that GCC or Clang can include in executables. They do basically the same thing as Valgrind, but should be much faster and since you are already compiling your own Lyx and Qt, the sanitizers should be a more appropriate tool. There are four sanitizers that you may want to compile with: asan, tsan, msan, ubsan; each for a different class of bugs. Instructions: include -fsanitize=asan (replace asan with wanted sanitizer if you want another one) to both compilation and linking commands (GCC or Clang) of all libraries and code. It is also a good idea to disable some compiler optimizations, in particular one should use -fno-omit-frame-pointer. See https://github.com/google/sanitizers/wiki for more information. By default the error reports will be output to stderr, but it is possible to output them to a file by setting an environment variable (search for log_path in https://github.com/google/sanitizers/wiki/AddressSanitizerFlags if in need of that option). I hope that was helpful. Regards, Neven Sajko -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On Tue, Feb 18, 2020 at 07:33:39PM -0500, Richard Kimberly Heck wrote: > On 2/18/20 6:07 PM, Scott Kostyshak wrote: > > Valgrind gave me the following error: > > > > ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely > > lost in loss record 5,165 of 5,862 > > ==732==at 0x483AE63: operator new(unsigned long) (in > > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > > ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const > > (Buffer.cpp:661) > > ==732==by 0x11E583C: lyx::(anonymous > > namespace)::copyToTempBuffer(lyx::ParagraphList const&, > > std::shared_ptr) (CutAndPaste.cpp:582) > > ==732==by 0x11E5C6A: lyx::(anonymous > > namespace)::putClipboard(lyx::ParagraphList const&, > > std::shared_ptr, > > std::__cxx11::basic_string, > > std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613) > > ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, > > std::__cxx11::basic_string, > > std::allocator > const&) (CutAndPaste.cpp:1123) > > ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) > > (CutAndPaste.cpp:1024) > > ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, > > lyx::FuncRequest&) (Text3.cpp:1593) > > ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, > > lyx::FuncRequest&) (InsetText.cpp:339) > > ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, > > lyx::FuncRequest&) (Inset.cpp:325) > > ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&) > > (Cursor.cpp:825) > > ==732==by 0x1816F4F: > > lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, > > lyx::DispatchResult&) (GuiView.cpp:3878) > > ==732==by 0x181B959: > > lyx::frontend::GuiView::dispatch(lyx::FuncRequest const&, > > lyx::DispatchResult&) (GuiView.cpp:4569) > > > > It comes from the following line (Buffer.cpp:661): > > > > cloned_buffers.push_back(new CloneList); > > > > Currently cloned_buffers is a list. Would it make sense to > > make it a list of *smart* pointers instead? Alternatively we could make a > > class and then make a custom destructor that would free the CloneLists that > > the list elements point to? > > This is some kind of thinko, probably on my part. The code at line 549 > was supposed to be cleaning this up, but it actually only removes the > entry from the list. > > If it works to make it a smart pointer of some kind, then that would be > simplest. But I think we could just do something like: > > else { > delete(*it); > cloned_buffers.erase(it); > } Ah that makes sense. > I should learn how to use valgrind. Then I could solve these problems > myself The hard part (for me) is interpreting the results. Well, actually the hardest part is waiting because LyX is very slow when run under valgrind. Probably there are some options to mess with to speed things up a bit. The command I run is this: valgrind --leak-check=full --track-origins=yes --log-file=valgrind.log ~/lyxbuilds/master/CMakeBuild/bin/lyx -userdir ~/lyxbuilds/master/user-dir Scott signature.asc Description: PGP signature -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak from list
On 2/18/20 6:07 PM, Scott Kostyshak wrote: > Valgrind gave me the following error: > > ==732== 112 (72 direct, 40 indirect) bytes in 1 blocks are definitely lost > in loss record 5,165 of 5,862 > ==732==at 0x483AE63: operator new(unsigned long) (in > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > ==732==by 0x103A62D: lyx::Buffer::cloneBufferOnly() const > (Buffer.cpp:661) > ==732==by 0x11E583C: lyx::(anonymous > namespace)::copyToTempBuffer(lyx::ParagraphList const&, > std::shared_ptr) (CutAndPaste.cpp:582) > ==732==by 0x11E5C6A: lyx::(anonymous > namespace)::putClipboard(lyx::ParagraphList const&, > std::shared_ptr, > std::__cxx11::basic_string, > std::allocator > const&, lyx::BufferParams) (CutAndPaste.cpp:613) > ==732==by 0x11E910B: lyx::cap::copySelection(lyx::Cursor const&, > std::__cxx11::basic_string, > std::allocator > const&) (CutAndPaste.cpp:1123) > ==732==by 0x11E84F8: lyx::cap::copySelection(lyx::Cursor const&) > (CutAndPaste.cpp:1024) > ==732==by 0x13ECDAB: lyx::Text::dispatch(lyx::Cursor&, > lyx::FuncRequest&) (Text3.cpp:1593) > ==732==by 0x1767E04: lyx::InsetText::doDispatch(lyx::Cursor&, > lyx::FuncRequest&) (InsetText.cpp:339) > ==732==by 0x15FFC39: lyx::Inset::dispatch(lyx::Cursor&, > lyx::FuncRequest&) (Inset.cpp:325) > ==732==by 0x11D1B13: lyx::Cursor::dispatch(lyx::FuncRequest const&) > (Cursor.cpp:825) > ==732==by 0x1816F4F: > lyx::frontend::GuiView::dispatchToBufferView(lyx::FuncRequest const&, > lyx::DispatchResult&) (GuiView.cpp:3878) > ==732==by 0x181B959: lyx::frontend::GuiView::dispatch(lyx::FuncRequest > const&, lyx::DispatchResult&) (GuiView.cpp:4569) > > It comes from the following line (Buffer.cpp:661): > > cloned_buffers.push_back(new CloneList); > > Currently cloned_buffers is a list. Would it make sense to make > it a list of *smart* pointers instead? Alternatively we could make a class > and then make a custom destructor that would free the CloneLists that the > list elements point to? This is some kind of thinko, probably on my part. The code at line 549 was supposed to be cleaning this up, but it actually only removes the entry from the list. If it works to make it a smart pointer of some kind, then that would be simplest. But I think we could just do something like: else { delete(*it); cloned_buffers.erase(it); } I should learn how to use valgrind. Then I could solve these problems myself Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Memory leak in master?
Hi Guillaume, I thought about it again, and you are right, of course. JMarc Le 24 février 2017 22:10:17 GMT+01:00, Guillaume Muncha écrit : >Le 22/02/2017 à 18:03, Jean-Marc Lasgouttes a écrit : >> It would be nice to hide these subtleties in the Cache >implementation, >> if possible. >> >I am not sure what you mean with "hide these subtleties". The >specification is simple: if the key does not exist in the cache, you >get >the default object.
Re: Memory leak in master?
Le 22/02/2017 à 18:03, Jean-Marc Lasgouttes a écrit : Le 21/02/2017 à 20:13, Guillaume Munch a écrit : BTW, why don't you use Cache::contains in getLayout like you do for other cache uses? This is explained in the documentation of Cache::object. It is enough to check for a null pointer in that case. It would be nice to hide these subtleties in the Cache implementation, if possible. Hi Jean-Marc, I am not sure what you mean with "hide these subtleties". The specification is simple: if the key does not exist in the cache, you get the default object. I thought about using boost::optional at first but for the main use I had in mind (shared_ptr) I found that the code would be heavier (you end up having to dereference twice) for little clarity gain. Guillaume
Re: Memory leak in master?
Le 23/02/2017 à 18:05, Richard Heck a écrit : Here is for reference the updated patch (even with comment clarification). Richard? OK. I'll test it over the next couple weeks, and if all is well move toward 2.2.3. rh Done at 998c3e7c8ef. There is no status.22x entry, since this is a fix over a new feature. JMarc
Re: Memory leak in master?
On 02/23/2017 04:46 AM, Jean-Marc Lasgouttes wrote: > Le 22/02/2017 à 18:03, Jean-Marc Lasgouttes a écrit : >> Indeed. Richard, I think this patch should go in 2.2.3, because the >> caching code that is in stable causes bad memory leaks with Qt5. > > Here is for reference the updated patch (even with comment > clarification). Richard? OK. I'll test it over the next couple weeks, and if all is well move toward 2.2.3. rh
Re: Memory leak in master?
On 02/23/2017 04:46 AM, Jean-Marc Lasgouttes wrote: > Le 22/02/2017 à 18:03, Jean-Marc Lasgouttes a écrit : >> Indeed. Richard, I think this patch should go in 2.2.3, because the >> caching code that is in stable causes bad memory leaks with Qt5. > > Here is for reference the updated patch (even with comment > clarification). Richard? OK. I'll test it over the next couple weeks. rh
Re: Memory leak in master?
Le 22/02/2017 à 18:03, Jean-Marc Lasgouttes a écrit : Indeed. Richard, I think this patch should go in 2.2.3, because the caching code that is in stable causes bad memory leaks with Qt5. Here is for reference the updated patch (even with comment clarification). Richard? JMarc From 8e04248b2fea58a7edcf16fb3318c302b04c66e8 Mon Sep 17 00:00:00 2001 From: Guillaume MunchDate: Mon, 20 Feb 2017 23:59:24 +0100 Subject: [PATCH] Introduce support/Cache.h Useful to cache copies of objects, including shared_ptrs. No risks of dangling pointer, and avoid naked pointers in the source. Fix memory leak when compiling with Qt5. As part as the backport to stable, this code has been change to work with C++98. (cherry picked from commit 33b696c8acf2e64b44d449180781de6dbc203709) (cherry picked from commit e04079aa528ecbf4a8e39ed2b19c3cb50174e151) (cherry picked from commit 5211ca52cac2ad7a6669d15c39f2cee172d18323) (cherry picked from commit 8353a53cc38fe364bee516e86a08251e4ae974fc) --- src/frontends/qt4/GuiFontMetrics.cpp | 126 +++--- src/frontends/qt4/GuiFontMetrics.h | 46 +++-- src/frontends/qt4/GuiPainter.cpp |3 +- src/support/Cache.h | 89 src/support/Makefile.am |1 + 5 files changed, 160 insertions(+), 105 deletions(-) create mode 100644 src/support/Cache.h diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index 5bb99aa..80ff819 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -24,14 +24,11 @@ #include "support/convert.h" #include "support/lassert.h" -#ifdef CACHE_SOME_METRICS #include -#endif using namespace std; using namespace lyx::support; -#ifdef CACHE_SOME_METRICS namespace std { /* @@ -46,11 +43,28 @@ uint qHash(lyx::docstring const & s) } } -#endif namespace lyx { namespace frontend { + +/* + * Limit (strwidth|breakat)_cache_ size to 512kB of string data. + * Limit qtextlayout_cache_ size to 500 elements (we do not know the + * size of the QTextLayout objects anyway). + * Note that all these numbers are arbitrary. + * Also, setting size to 0 is tantamount to disabling the cache. + */ +int cache_metrics_width_size = 1 << 19; +int cache_metrics_breakat_size = 1 << 19; +// Qt 5.x already has its own caching of QTextLayout objects +#if (QT_VERSION < 0x05) +int cache_metrics_qtextlayout_size = 500; +#else +int cache_metrics_qtextlayout_size = 0; +#endif + + namespace { /** * Convert a UCS4 character into a QChar. @@ -73,23 +87,11 @@ inline QChar const ucs4_to_qchar(char_type const ucs4) } // anon namespace -/* - * Limit (strwidth|breakat)_cache_ size to 512kB of string data. - * Limit qtextlayout_cache_ size to 500 elements (we do not know the - * size of the QTextLayout objects anyway). - * Note that all these numbers are arbitrary. - */ GuiFontMetrics::GuiFontMetrics(QFont const & font) - : font_(font), metrics_(font, 0) -#ifdef CACHE_METRICS_WIDTH - , strwidth_cache_(1 << 19) -#endif -#ifdef CACHE_METRICS_BREAKAT - , breakat_cache_(1 << 19) -#endif -#ifdef CACHE_METRICS_QTEXTLAYOUT - , qtextlayout_cache_(500) -#endif + : font_(font), metrics_(font, 0), + strwidth_cache_(cache_metrics_width_size), + breakat_cache_(cache_metrics_breakat_size), + qtextlayout_cache_(cache_metrics_qtextlayout_size) { } @@ -174,11 +176,8 @@ int GuiFontMetrics::rbearing(char_type c) const int GuiFontMetrics::width(docstring const & s) const { -#ifdef CACHE_METRICS_WIDTH - int * pw = strwidth_cache_[s]; - if (pw) - return *pw; -#endif + if (strwidth_cache_.contains(s)) + return strwidth_cache_[s]; /* For some reason QMetrics::width returns a wrong value with Qt5 * with some arabic text. OTOH, QTextLayout is broken for single * characters with null width (like \not in mathed). Also, as a @@ -200,9 +199,7 @@ int GuiFontMetrics::width(docstring const & s) const tl.endLayout(); w = int(line.naturalTextWidth()); } -#ifdef CACHE_METRICS_WIDTH - strwidth_cache_.insert(s, new int(w), s.size() * sizeof(char_type)); -#endif + strwidth_cache_.insert(s, w, s.size() * sizeof(char_type)); return w; } @@ -225,31 +222,26 @@ int GuiFontMetrics::signedWidth(docstring const & s) const } -QTextLayout const * +shared_ptr GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, double const wordspacing) const { - QTextLayout * ptl; -#ifdef CACHE_METRICS_QTEXTLAYOUT - docstring const s_cache = s + (rtl ? "r" : "l") + convert(wordspacing); - ptl = qtextlayout_cache_[s_cache]; - if (!ptl) { -#endif - ptl = new QTextLayout(); - ptl->setCacheEnabled(true); - ptl->setText(toqstr(s)); - QFont copy = font_; - copy.setWordSpacing(wordspacing); - ptl->setFont(copy); - // Note that both setFlags and the enums are undocumented - ptl->setFlags(rtl ? Qt::TextForceRightToLeft : Qt::TextForceLeftToRight); -
Re: Memory leak in master?
Le 21/02/2017 à 20:13, Guillaume Munch a écrit : Thanks for doing this. I thought there was some bigger adaptation to the code to do but indeed it only looks like a matter of C++98 conversion. You mean it was a trap? It did not work %-p I think it's all good except: class Cache : private QCache{ -#if !(defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)) +#if defined(LYX_USE_CXX11) && !(defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)) Indeed. Richard, I think this patch should go in 2.2.3, because the caching code that is in stable causes bad memory leaks with Qt5. BTW, why don't you use Cache::contains in getLayout like you do for other cache uses? This is explained in the documentation of Cache::object. It is enough to check for a null pointer in that case. It would be nice to hide these subtleties in the Cache implementation, if possible. JMarc
Re: Memory leak in master?
Le 21/02/2017 à 07:19, Jean-Marc Lasgouttes a écrit : Le 21/02/2017 à 00:08, Guillaume Munch a écrit : Could you do the backport to stable? :) What about that? Please check especially the code related to LYX_USE_CXX11 in Cache.h. For the caching, I re-read the code to make sure that my hand-merging was correct, I hope I did not miss anything. Thanks for doing this. I thought there was some bigger adaptation to the code to do but indeed it only looks like a matter of C++98 conversion. I think it's all good except: class Cache : private QCache{ -#if !(defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)) +#if defined(LYX_USE_CXX11) && !(defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)) BTW, why don't you use Cache::contains in getLayout like you do for other cache uses? This is explained in the documentation of Cache::object. It is enough to check for a null pointer in that case. Guillaume
Re: Memory leak in master?
Le 20/02/2017 à 18:25, Jean-Marc Lasgouttes a écrit : Le 10/02/2017 à 17:58, Guillaume Munch a écrit : There's more data, but I am not sure how to interpret the results that massif-visualizer shows. If you send the file or make it available, I can take a look. Here it is. But if it is like callgrind, it might lack the symbols. Interestingly, the massif output that you sent also points to MathMacro::clone(). Besides the obvious getTextLayout leak that needs to be plugged, would you have an idea on why it is specifically macros that appear here (with an increasing memory use, like the other)? I do not see other math objects doing that. Note that there are other sources that point to MathAtom::MathAtom, as the drop in percentage shows. Interesting, massif-visualizer shows the results differently. With it one sees that MathAtom quickly reaches its final space usage and even decreases at some point. It does not look similar to the slow creep of the memory leak. Again, I am not sure that I know how to interpret the results. Guillaume
Re: Memory leak in master?
Le 21/02/2017 à 00:08, Guillaume Munch a écrit : Could you do the backport to stable? :) What about that? Please check especially the code related to LYX_USE_CXX11 in Cache.h. For the caching, I re-read the code to make sure that my hand-merging was correct, I hope I did not miss anything. BTW, why don't you use Cache::contains in getLayout like you do for other cache uses? JMarc From 30b62647ba2dc111aeb4425292e92994cf96f931 Mon Sep 17 00:00:00 2001 From: Guillaume MunchDate: Mon, 20 Feb 2017 23:59:24 +0100 Subject: [PATCH] Introduce support/Cache.h Useful to cache copies of objects, including shared_ptrs. No risks of dangling pointer, and avoid naked pointers in the source. Fix memory leak when compiling with Qt5. As part as the backport to stable, this code has been change to work with C++98. (cherry picked from commit 33b696c8acf2e64b44d449180781de6dbc203709) (cherry picked from commit e04079aa528ecbf4a8e39ed2b19c3cb50174e151) (cherry picked from commit 5211ca52cac2ad7a6669d15c39f2cee172d18323) --- src/frontends/qt4/GuiFontMetrics.cpp | 126 +++--- src/frontends/qt4/GuiFontMetrics.h | 46 +++-- src/frontends/qt4/GuiPainter.cpp |3 +- src/support/Cache.h | 88 src/support/Makefile.am |1 + 5 files changed, 159 insertions(+), 105 deletions(-) create mode 100644 src/support/Cache.h diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index 5bb99aa..80ff819 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -24,14 +24,11 @@ #include "support/convert.h" #include "support/lassert.h" -#ifdef CACHE_SOME_METRICS #include -#endif using namespace std; using namespace lyx::support; -#ifdef CACHE_SOME_METRICS namespace std { /* @@ -46,11 +43,28 @@ uint qHash(lyx::docstring const & s) } } -#endif namespace lyx { namespace frontend { + +/* + * Limit (strwidth|breakat)_cache_ size to 512kB of string data. + * Limit qtextlayout_cache_ size to 500 elements (we do not know the + * size of the QTextLayout objects anyway). + * Note that all these numbers are arbitrary. + * Also, setting size to 0 is tantamount to disabling the cache. + */ +int cache_metrics_width_size = 1 << 19; +int cache_metrics_breakat_size = 1 << 19; +// Qt 5.x already has its own caching of QTextLayout objects +#if (QT_VERSION < 0x05) +int cache_metrics_qtextlayout_size = 500; +#else +int cache_metrics_qtextlayout_size = 0; +#endif + + namespace { /** * Convert a UCS4 character into a QChar. @@ -73,23 +87,11 @@ inline QChar const ucs4_to_qchar(char_type const ucs4) } // anon namespace -/* - * Limit (strwidth|breakat)_cache_ size to 512kB of string data. - * Limit qtextlayout_cache_ size to 500 elements (we do not know the - * size of the QTextLayout objects anyway). - * Note that all these numbers are arbitrary. - */ GuiFontMetrics::GuiFontMetrics(QFont const & font) - : font_(font), metrics_(font, 0) -#ifdef CACHE_METRICS_WIDTH - , strwidth_cache_(1 << 19) -#endif -#ifdef CACHE_METRICS_BREAKAT - , breakat_cache_(1 << 19) -#endif -#ifdef CACHE_METRICS_QTEXTLAYOUT - , qtextlayout_cache_(500) -#endif + : font_(font), metrics_(font, 0), + strwidth_cache_(cache_metrics_width_size), + breakat_cache_(cache_metrics_breakat_size), + qtextlayout_cache_(cache_metrics_qtextlayout_size) { } @@ -174,11 +176,8 @@ int GuiFontMetrics::rbearing(char_type c) const int GuiFontMetrics::width(docstring const & s) const { -#ifdef CACHE_METRICS_WIDTH - int * pw = strwidth_cache_[s]; - if (pw) - return *pw; -#endif + if (strwidth_cache_.contains(s)) + return strwidth_cache_[s]; /* For some reason QMetrics::width returns a wrong value with Qt5 * with some arabic text. OTOH, QTextLayout is broken for single * characters with null width (like \not in mathed). Also, as a @@ -200,9 +199,7 @@ int GuiFontMetrics::width(docstring const & s) const tl.endLayout(); w = int(line.naturalTextWidth()); } -#ifdef CACHE_METRICS_WIDTH - strwidth_cache_.insert(s, new int(w), s.size() * sizeof(char_type)); -#endif + strwidth_cache_.insert(s, w, s.size() * sizeof(char_type)); return w; } @@ -225,31 +222,26 @@ int GuiFontMetrics::signedWidth(docstring const & s) const } -QTextLayout const * +shared_ptr GuiFontMetrics::getTextLayout(docstring const & s, bool const rtl, double const wordspacing) const { - QTextLayout * ptl; -#ifdef CACHE_METRICS_QTEXTLAYOUT - docstring const s_cache = s + (rtl ? "r" : "l") + convert(wordspacing); - ptl = qtextlayout_cache_[s_cache]; - if (!ptl) { -#endif - ptl = new QTextLayout(); - ptl->setCacheEnabled(true); - ptl->setText(toqstr(s)); - QFont copy = font_; - copy.setWordSpacing(wordspacing); - ptl->setFont(copy); - // Note that both setFlags and the enums are undocumented - ptl->setFlags(rtl ?
Re: Memory leak in master?
Le 21/02/2017 à 00:08, Guillaume Munch a écrit : Le 20/02/2017 à 18:27, Jean-Marc Lasgouttes a écrit : Le 13/02/2017 à 20:55, Jean-Marc Lasgouttes a écrit : What I mean is that you can easily force QCache into shared ownership by replacing QCachewith QCache and create the appropriate wrappers for insertion and retrieval. Could you have a go at this solution? Could you do the backport to stable? :) You mean the version that does not require C++11? Mmm, I can always have a go at it. JMarc
Re: Memory leak in master?
Le 20/02/2017 à 18:27, Jean-Marc Lasgouttes a écrit : Le 13/02/2017 à 20:55, Jean-Marc Lasgouttes a écrit : What I mean is that you can easily force QCache into shared ownership by replacing QCachewith QCache and create the appropriate wrappers for insertion and retrieval. Could you have a go at this solution? Could you do the backport to stable? :)
Re: Memory leak in master?
Le 13/02/2017 à 20:55, Jean-Marc Lasgouttes a écrit : What I mean is that you can easily force QCache into shared ownership by replacing QCachewith QCache and create the appropriate wrappers for insertion and retrieval. Could you have a go at this solution? JMarc
Re: Memory leak in master?
Le 10/02/2017 à 17:58, Guillaume Munch a écrit : There's more data, but I am not sure how to interpret the results that massif-visualizer shows. If you send the file or make it available, I can take a look. Here it is. But if it is like callgrind, it might lack the symbols. Interestingly, the massif output that you sent also points to MathMacro::clone(). Besides the obvious getTextLayout leak that needs to be plugged, would you have an idea on why it is specifically macros that appear here (with an increasing memory use, like the other)? I do not see other math objects doing that. Note that there are other sources that point to MathAtom::MathAtom, as the drop in percentage shows. JMarc ->09.75% (18,139,776B) 0x8DA8B5: lyx::MathMacro::MathMacro(lyx::MathMacro const&) (MathMacro.cpp:299) | ->09.75% (18,139,776B) 0x8DB03F: lyx::MathMacro::clone() const (MathMacro.cpp:394) | ->09.75% (18,139,776B) 0x8AD26D: lyx::MathAtom::MathAtom(lyx::MathAtom const&) (MathAtom.cpp:35) | ->03.42% (6,356,216B) 0x86ADFE: std::vector::operator=(std::vector const&) (stl_construct.h:75) | | ->01.16% (2,158,056B) 0x8EF837: lyx::(anonymous namespace)::Parser::parse(lyx::MathData&, unsigned int, lyx::Inset::mode_type) (vector:111) | | | ->01.16% (2,158,056B) 0x8EFC99: lyx::mathed_parse_cell(lyx::MathData&, lyx::docstring const&, lyx::Parse::flags) (MathParser.cpp:2150) | | | | ->01.16% (2,158,056B) 0x9079EB: lyx::asArray(lyx::docstring const&, lyx::MathData&, lyx::Parse::flags) (MathSupport.cpp:985) | | | | ->01.16% (2,158,056B) 0x8DBBE8: lyx::MathMacro::updateRepresentation(lyx::Cursor*, lyx::MacroContext const&, lyx::UpdateType, int) (MathMacro.cpp:699)
Re: Memory leak with WordList
Le 31/12/2016 à 15:42, Guillaume Munch a écrit : Snippet of Valgrind output for stable below. The WordList object created in theWordList() never gets deleted, although QThreadStorage is supposed to be automagic. According to the docs, the GlobalWordLists were deleted with the QApplication, that is, before the call to cleanUpWordLists. Thanks for fixing this. JMarc
Re: Memory leak with WordList
Le 31/12/2016 à 13:16, Jean-Marc Lasgouttes a écrit : WordList leaks all its contents on exit, which is very impolite (even though the memory will be reclaimed anyway). This hides real memory leaks that may happen elsewhere. Snippet of Valgrind output for stable below. The WordList object created in theWordList() never gets deleted, although QThreadStorage is supposed to be automagic. According to the docs, the GlobalWordLists were deleted with the QApplication, that is, before the call to cleanUpWordLists.
Re: Memory leak ?
Am 22.01.2012 um 21:58 schrieb Jean-Marc Lasgouttes: Le 22/01/12 02:09, Lars Gullik Bjønnes a écrit : My testing so far says the opposite. We are continually using more and more memory. Not especially fast, but it is there. A good tool to know where memory goes is the massif tool of valgrind. I've tried to use Instruments Leak-Detector on Mac. 1. It doesn't find any suspicious leaks - the growing memory is not lost. 2. The growing memory is allocated in TextClass constructor - about 40k per autosave operation. 3. Call Stack: Total % Bytes Library Symbol Name ... 11.41.65 MB lyx::frontend::GuiView::autoSave() 11.41.64 MB lyx::Buffer::cloneBufferOnly() const 11.41.64 MB lyx::Buffer::Buffer(std::string const, bool, lyx::Buffer const*) 11.41.64 MB lyx::Buffer::Impl::Impl(lyx::Buffer*, lyx::support::FileName const, bool, lyx::Buffer const*) 11.41.64 MB lyx::BufferParams::BufferParams() 11.41.64 MB lyx::BufferParams::makeDocumentClass() 11.41.64 MB lyx::DocumentClassBundle::makeDocumentClass(lyx::LayoutFile const, lyx::LayoutModuleList const) 11.41.64 MB lyx::DocumentClassBundle::newClass(lyx::LayoutFile const) 11.21.61 MB lyx::DocumentClass::DocumentClass(lyx::LayoutFile const) 11.21.61 MB lyx::TextClass::TextClass(lyx::TextClass const) 1 minute later Total % Bytes Library Symbol Name ... 11.71.69 MB lyx::DocumentClassBundle::newClass(lyx::LayoutFile const) 11.41.65 MB lyx::TextClass::TextClass(lyx::TextClass const) Stephan
Re: Memory leak ?
Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass -01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. JMarc
Re: Memory leak ?
On 01/23/2012 05:53 AM, Jean-Marc Lasgouttes wrote: Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass -01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. I think I understand what's going on here. We release the Buffers, but we do not, it is true, release the TextClass objects that are associated with them. There's actually a good reason for that where ordinary Buffers are concerned---there could be something on the cut stack that still has a reference to the TextClass in question---but in the case of AutoSave, there is no reason we need to keep these around. See the comments at the end of TextClass.h, concerning the DocumentClassBundle. I'll try to have a look shortly, but classes start again on Wednesday. If anyone else had ideas, I'd be happy to hear them. Richard
Re: Memory leak ?
Op 23-1-2012 15:54, Richard Heck schreef: On 01/23/2012 05:53 AM, Jean-Marc Lasgouttes wrote: Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass -01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. I think I understand what's going on here. We release the Buffers, but we do not, it is true, release the TextClass objects that are associated with them. There's actually a good reason for that where ordinary Buffers are concerned---there could be something on the cut stack that still has a reference to the TextClass in question---but in the case of AutoSave, there is no reason we need to keep these around. See the comments at the end of TextClass.h, concerning the DocumentClassBundle. I'll try to have a look shortly, but classes start again on Wednesday. If anyone else had ideas, I'd be happy to hear them. Richard We don't need a new TextClass everytime. If we already have one for the documentclass/layout, we can reuse it again. This also means that with every preview we lose memory. Vincent
Re: Memory leak ?
On 01/23/2012 12:04 PM, Vincent van Ravesteijn wrote: Op 23-1-2012 15:54, Richard Heck schreef: On 01/23/2012 05:53 AM, Jean-Marc Lasgouttes wrote: Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass -01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) -04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. I think I understand what's going on here. We release the Buffers, but we do not, it is true, release the TextClass objects that are associated with them. There's actually a good reason for that where ordinary Buffers are concerned---there could be something on the cut stack that still has a reference to the TextClass in question---but in the case of AutoSave, there is no reason we need to keep these around. See the comments at the end of TextClass.h, concerning the DocumentClassBundle. I'll try to have a look shortly, but classes start again on Wednesday. If anyone else had ideas, I'd be happy to hear them. Richard We don't need a new TextClass everytime. If we already have one for the documentclass/layout, we can reuse it again. I'm afraid it's more complicated than that. I mean, in principle we could do that, but the new TextClass (actually DocumentClass) object is created in the BufferParams constructor. I think I have a good idea what to do here, though. Minimally, we could delete the clone's DocumentClass on destruction since we know that no-one else will have a reference to it. This also means that with every preview we lose memory. Yes, this is all an unforeseen consequence of the cloning business. (Enrico strikes again!) But there are other places we create these objects and never get rid of them. Richard
Re: Memory leak ?
Am 22.01.2012 um 21:58 schrieb Jean-Marc Lasgouttes: > Le 22/01/12 02:09, Lars Gullik Bjønnes a écrit : >> My testing so far says the opposite. We are continually using more and >> more memory. Not especially fast, but it is there. > > A good tool to know where memory goes is the massif tool of valgrind. I've tried to use Instruments Leak-Detector on Mac. 1. It doesn't find any suspicious leaks - the growing memory is not lost. 2. The growing memory is allocated in TextClass constructor - about 40k per autosave operation. 3. Call Stack: Total % Bytes Library Symbol Name ... 11.41.65 MB lyx::frontend::GuiView::autoSave() 11.41.64 MB lyx::Buffer::cloneBufferOnly() const 11.41.64 MB lyx::Buffer::Buffer(std::string const&, bool, lyx::Buffer const*) 11.41.64 MB lyx::Buffer::Impl::Impl(lyx::Buffer*, lyx::support::FileName const&, bool, lyx::Buffer const*) 11.41.64 MB lyx::BufferParams::BufferParams() 11.41.64 MB lyx::BufferParams::makeDocumentClass() 11.41.64 MB lyx::DocumentClassBundle::makeDocumentClass(lyx::LayoutFile const&, lyx::LayoutModuleList const&) 11.41.64 MB lyx::DocumentClassBundle::newClass(lyx::LayoutFile const&) 11.21.61 MB lyx::DocumentClass::DocumentClass(lyx::LayoutFile const&) 11.21.61 MB lyx::TextClass::TextClass(lyx::TextClass const&) 1 minute later Total % Bytes Library Symbol Name ... 11.71.69 MB lyx::DocumentClassBundle::newClass(lyx::LayoutFile const&) 11.41.65 MB lyx::TextClass::TextClass(lyx::TextClass const&) Stephan
Re: Memory leak ?
Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass ->01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. JMarc
Re: Memory leak ?
On 01/23/2012 05:53 AM, Jean-Marc Lasgouttes wrote: Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass ->01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. I think I understand what's going on here. We release the Buffers, but we do not, it is true, release the TextClass objects that are associated with them. There's actually a good reason for that where ordinary Buffers are concerned---there could be something on the cut stack that still has a reference to the TextClass in question---but in the case of AutoSave, there is no reason we need to keep these around. See the comments at the end of TextClass.h, concerning the DocumentClassBundle. I'll try to have a look shortly, but classes start again on Wednesday. If anyone else had ideas, I'd be happy to hear them. Richard
Re: Memory leak ?
Op 23-1-2012 15:54, Richard Heck schreef: On 01/23/2012 05:53 AM, Jean-Marc Lasgouttes wrote: Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass ->01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. I think I understand what's going on here. We release the Buffers, but we do not, it is true, release the TextClass objects that are associated with them. There's actually a good reason for that where ordinary Buffers are concerned---there could be something on the cut stack that still has a reference to the TextClass in question---but in the case of AutoSave, there is no reason we need to keep these around. See the comments at the end of TextClass.h, concerning the DocumentClassBundle. I'll try to have a look shortly, but classes start again on Wednesday. If anyone else had ideas, I'd be happy to hear them. Richard We don't need a new TextClass everytime. If we already have one for the documentclass/layout, we can reuse it again. This also means that with every preview we lose memory. Vincent
Re: Memory leak ?
On 01/23/2012 12:04 PM, Vincent van Ravesteijn wrote: Op 23-1-2012 15:54, Richard Heck schreef: On 01/23/2012 05:53 AM, Jean-Marc Lasgouttes wrote: Le 23/01/2012 10:56, Lars Gullik Bjønnes a écrit : | A good tool to know where memory goes is the massif tool of valgrind. This is the result from a ~6hrs run. I am not quite sure how to read it. Like what Stephan reported, we see: fantomas: ms_print massif.out.24204|grep TextClass::TextClass ->01.02% (303,120B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.16% (972,720B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.06% (1,262,880B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->03.56% (1,486,080B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.12% (1,642,320B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.06% (1,709,280B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->05.74% (1,865,520B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) ->04.54% (1,932,480B) 0x58C82F: lyx::TextClass::TextClass(lyx::TextClass const&) (new_allocator.h:92) We see that the number of allocated TextClass objects increses. I think that we never release buffers cloned by autosave. I think I understand what's going on here. We release the Buffers, but we do not, it is true, release the TextClass objects that are associated with them. There's actually a good reason for that where ordinary Buffers are concerned---there could be something on the cut stack that still has a reference to the TextClass in question---but in the case of AutoSave, there is no reason we need to keep these around. See the comments at the end of TextClass.h, concerning the DocumentClassBundle. I'll try to have a look shortly, but classes start again on Wednesday. If anyone else had ideas, I'd be happy to hear them. Richard We don't need a new TextClass everytime. If we already have one for the documentclass/layout, we can reuse it again. I'm afraid it's more complicated than that. I mean, in principle we could do that, but the new TextClass (actually DocumentClass) object is created in the BufferParams constructor. I think I have a good idea what to do here, though. Minimally, we could delete the clone's DocumentClass on destruction since we know that no-one else will have a reference to it. This also means that with every preview we lose memory. Yes, this is all an unforeseen consequence of the cloning business. (Enrico strikes again!) But there are other places we create these objects and never get rid of them. Richard
Re: Memory leak ?
Richard Heck rgh...@comcast.net writes: | On 01/21/2012 08:09 PM, Lars Gullik Bjønnes wrote: Richard Heckrgh...@comcast.net writes: | On 01/21/2012 05:36 AM, Jean-Pierre Chrétien wrote: Hello, A French user tells me privately that he finds a big memeory leak in LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). On my Debian Squeeze and a simple document, I find about 1.7ko/mn. Testing with the Userguide, I find around 19ko/mn with the following recipe: - open LyX-2.0.2 (English locale) - open the UserGuide - minimize the window - activate a crontab every minute with command ps -o rss= -p process_id | Below is a log of a test on Fedora 16, Qt 4.8.0, with 2.0.3svn, | compiled as for release. As you'll see, there's a jump when the first | autosave happens---we may be loading some Qt libraries we weren't | otherwise using---but after that, there's not much change. My testing so far says the opposite. We are continually using more and more memory. Not especially fast, but it is there. I see a steady increase. Plot the attached with gnuplot (plot lyx-leak.dat using 1:2) and see for yourself. Snarfed with let a=0; while true; do echo -n $a lyx-leak.dat; ps -o rss=,vsz= -p 32013 lyx-leak.dat ; let ++a; sleep 1 ; done | Hard to know what the difference is here. Was this with branch or trunk? Trunk. -- Lgb
Re: Memory leak ?
Le 22/01/12 02:09, Lars Gullik Bjønnes a écrit : My testing so far says the opposite. We are continually using more and more memory. Not especially fast, but it is there. A good tool to know where memory goes is the massif tool of valgrind. JMarc
Re: Memory leak ?
Richard Heckwrites: | On 01/21/2012 08:09 PM, Lars Gullik Bjønnes wrote: >> Richard Heck writes: >> >> | On 01/21/2012 05:36 AM, Jean-Pierre Chrétien wrote: Hello, A French user tells me privately that he finds a big memeory leak in LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). On my Debian Squeeze and a simple document, I find about 1.7ko/mn. Testing with the Userguide, I find around 19ko/mn with the following recipe: - open LyX-2.0.2 (English locale) - open the UserGuide - minimize the window - activate a crontab every minute with command ps -o rss= -p >> | Below is a log of a test on Fedora 16, Qt 4.8.0, with 2.0.3svn, >> | compiled as for release. As you'll see, there's a jump when the first >> | autosave happens---we may be loading some Qt libraries we weren't >> | otherwise using---but after that, there's not much change. >> >> My testing so far says the opposite. We are continually using more and >> more memory. Not especially fast, but it is there. >> >> I see a steady increase. >> >> Plot the attached with gnuplot >> (plot "lyx-leak.dat" using 1:2) >> >> and see for yourself. Snarfed with >> >> let a=0; while true; do echo -n "$a ">> lyx-leak.dat; ps -o rss=,vsz= -p >> 32013>> lyx-leak.dat ; let ++a; sleep 1 ; done | Hard to know what the difference is here. Was this with branch or trunk? Trunk. -- Lgb
Re: Memory leak ?
Le 22/01/12 02:09, Lars Gullik Bjønnes a écrit : My testing so far says the opposite. We are continually using more and more memory. Not especially fast, but it is there. A good tool to know where memory goes is the massif tool of valgrind. JMarc
Re: Memory leak ?
Jean-Pierre Chrétien jeanpierre.chretien at free.fr writes: I find no leak with Lyx-1.6.10, and an much smaller one (identical for each) of 0.28ko/mn with Lyx-2.0.2 and 2.0.1. I meant Lyx-2.0.0 and 2.0.1, sorry. -- Jean-Pierre
Re: Memory leak ?
Jean-Pierre Chrétien jeanpierre.chret...@free.fr writes: | Hello, | A French user tells me privately that he finds a big memeory leak in | LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). | On my Debian Squeeze and a simple document, I find about 1.7ko/mn. | Testing with the Userguide, I find around 19ko/mn with the following recipe: | - open LyX-2.0.2 (English locale) | - open the UserGuide | - minimize the window | - activate a crontab every minute with command ps -o rss= -p process_id | I find no leak with Lyx-1.6.10, and an much smaller one (identical for | each) of 0.28ko/mn with Lyx-2.0.2 and 2.0.1. | Is this a real leak, a misleading procedure, or a new behaviour with | 2.0 (hidden tasks like auto-update or something) ? I ran valgrind on trunk I couldn't see anything special. I also left lyx arunning for a long time no valgrind reports. So either the leak you see does not exist on trunk, or it is not a leak in the strict sense. That however does not mean that what you see is wrong. I used this suppression file when running whith valgrind. Some of the suppressions should be looked into, but they are very minor leaks. lyx.supp Description: Binary data -- Lgb
Re: Memory leak ?
On 01/21/2012 05:36 AM, Jean-Pierre Chrétien wrote: Hello, A French user tells me privately that he finds a big memeory leak in LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). On my Debian Squeeze and a simple document, I find about 1.7ko/mn. Testing with the Userguide, I find around 19ko/mn with the following recipe: - open LyX-2.0.2 (English locale) - open the UserGuide - minimize the window - activate a crontab every minute with command ps -o rss= -p process_id Below is a log of a test on Fedora 16, Qt 4.8.0, with 2.0.3svn, compiled as for release. As you'll see, there's a jump when the first autosave happens---we may be loading some Qt libraries we weren't otherwise using---but after that, there's not much change. Richard == /home/rgheck/ while [ ss ]; do ps -o rss= -p 3483; sleep 60; done 63512 63416 63416 63416 63416 63416 Now I will make the document dirty. 63516 63516 Autosave is set to 10 minutes 63628 63628 73444 /tmp/ ll *User* -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:17 #UserGuide.lyx# -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:03 UserGuide.lyx 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73480 /tmp/ ll *User* -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:37 #UserGuide.lyx# -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:03 UserGuide.lyx 73464 73464 73704 73704 74392 74392 74392 74392 74392 74568 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 74568
Re: Memory leak ?
On 01/21/2012 08:09 PM, Lars Gullik Bjønnes wrote: Richard Heckrgh...@comcast.net writes: | On 01/21/2012 05:36 AM, Jean-Pierre Chrétien wrote: Hello, A French user tells me privately that he finds a big memeory leak in LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). On my Debian Squeeze and a simple document, I find about 1.7ko/mn. Testing with the Userguide, I find around 19ko/mn with the following recipe: - open LyX-2.0.2 (English locale) - open the UserGuide - minimize the window - activate a crontab every minute with command ps -o rss= -p process_id | Below is a log of a test on Fedora 16, Qt 4.8.0, with 2.0.3svn, | compiled as for release. As you'll see, there's a jump when the first | autosave happens---we may be loading some Qt libraries we weren't | otherwise using---but after that, there's not much change. My testing so far says the opposite. We are continually using more and more memory. Not especially fast, but it is there. I see a steady increase. Plot the attached with gnuplot (plot lyx-leak.dat using 1:2) and see for yourself. Snarfed with let a=0; while true; do echo -n $a lyx-leak.dat; ps -o rss=,vsz= -p 32013 lyx-leak.dat ; let ++a; sleep 1 ; done Hard to know what the difference is here. Was this with branch or trunk? Richard
Re: Memory leak ?
Jean-Pierre Chrétien free.fr> writes: > I find no leak with Lyx-1.6.10, and an much smaller one (identical for each) > of 0.28ko/mn with Lyx-2.0.2 and 2.0.1. I meant Lyx-2.0.0 and 2.0.1, sorry. -- Jean-Pierre
Re: Memory leak ?
Jean-Pierre Chrétienwrites: | Hello, > | A French user tells me privately that he finds a big memeory leak in | LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). > | On my Debian Squeeze and a simple document, I find about 1.7ko/mn. > | Testing with the Userguide, I find around 19ko/mn with the following recipe: | - open LyX-2.0.2 (English locale) | - open the UserGuide | - minimize the window | - activate a crontab every minute with command ps -o rss= -p > | I find no leak with Lyx-1.6.10, and an much smaller one (identical for | each) of 0.28ko/mn with Lyx-2.0.2 and 2.0.1. > | Is this a real leak, a misleading procedure, or a new behaviour with | 2.0 (hidden tasks like auto-update or something) ? I ran valgrind on trunk I couldn't see anything special. I also left lyx arunning for a long time no valgrind reports. So either the leak you see does not exist on trunk, or it is not a leak in the strict sense. That however does not mean that what you see is wrong. I used this suppression file when running whith valgrind. Some of the suppressions should be looked into, but they are very minor leaks. lyx.supp Description: Binary data -- Lgb
Re: Memory leak ?
On 01/21/2012 05:36 AM, Jean-Pierre Chrétien wrote: Hello, A French user tells me privately that he finds a big memeory leak in LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). On my Debian Squeeze and a simple document, I find about 1.7ko/mn. Testing with the Userguide, I find around 19ko/mn with the following recipe: - open LyX-2.0.2 (English locale) - open the UserGuide - minimize the window - activate a crontab every minute with command ps -o rss= -p Below is a log of a test on Fedora 16, Qt 4.8.0, with 2.0.3svn, compiled as for release. As you'll see, there's a jump when the first autosave happens---we may be loading some Qt libraries we weren't otherwise using---but after that, there's not much change. Richard == /home/rgheck/ > while [ "ss" ]; do ps -o rss= -p 3483; sleep 60; done 63512 63416 63416 63416 63416 63416 Now I will make the document dirty. 63516 63516 Autosave is set to 10 minutes 63628 63628 73444 /tmp/ > ll *User* -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:17 #UserGuide.lyx# -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:03 UserGuide.lyx 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73476 73480 /tmp/ > ll *User* -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:37 #UserGuide.lyx# -rw-rw-r--. 1 rgheck rgheck 795K Jan 21 12:03 UserGuide.lyx 73464 73464 73704 73704 74392 74392 74392 74392 74392 74568 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 73888 74568
Re: Memory leak ?
On 01/21/2012 08:09 PM, Lars Gullik Bjønnes wrote: Richard Heckwrites: | On 01/21/2012 05:36 AM, Jean-Pierre Chrétien wrote: Hello, A French user tells me privately that he finds a big memeory leak in LyX-2.0.2 on an Archlinux LyX distribution (about 1.5Mo/mn). On my Debian Squeeze and a simple document, I find about 1.7ko/mn. Testing with the Userguide, I find around 19ko/mn with the following recipe: - open LyX-2.0.2 (English locale) - open the UserGuide - minimize the window - activate a crontab every minute with command ps -o rss= -p | Below is a log of a test on Fedora 16, Qt 4.8.0, with 2.0.3svn, | compiled as for release. As you'll see, there's a jump when the first | autosave happens---we may be loading some Qt libraries we weren't | otherwise using---but after that, there's not much change. My testing so far says the opposite. We are continually using more and more memory. Not especially fast, but it is there. I see a steady increase. Plot the attached with gnuplot (plot "lyx-leak.dat" using 1:2) and see for yourself. Snarfed with let a=0; while true; do echo -n "$a ">> lyx-leak.dat; ps -o rss=,vsz= -p 32013>> lyx-leak.dat ; let ++a; sleep 1 ; done Hard to know what the difference is here. Was this with branch or trunk? Richard
Re: Memory leak?
rgheck wrote: On 08/10/2009 05:01 PM, Abdelrazak Younes wrote: On 10/08/2009 22:46, rgheck wrote: On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_-image()-clone()); bool const success = image_-setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? We can, if you manage to give access to the original image inside GuiImage::setPixmap(). Without major surgery I am sure this is possible. Or at least that was the conclusion I reached last time I had the same idea ;-) My thought was more simpleminded, but I don't know this code. Why do we need a cache of the original image once the pixmap has been set? If the original is changed, then presumably we want to reload. But if it doesn't change, we don't need access to it any more, do we? Correct. So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? You can try. Last time I did, I failed. But I was so upset about this piece of code that maybe the solution was much simpler than I thought... Abdel.
Re: Memory leak?
On 08/11/2009 03:40 AM, Abdelrazak Younes wrote: So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? You can try. Last time I did, I failed. But I was so upset about this piece of code that maybe the solution was much simpler than I thought... Hmm. That's a little too brute force. We need the cache item, for monitoring and the like. What we don't need is to have the image itself still there. Does that seem right? (Sorry, I don't know this code and am just poking around.) rh
Re: Memory leak?
rgheck wrote: On 08/10/2009 05:01 PM, Abdelrazak Younes wrote: On 10/08/2009 22:46, rgheck wrote: On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_->image()->clone()); bool const success = image_->setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? We can, if you manage to give access to the original image inside GuiImage::setPixmap(). Without major surgery I am sure this is possible. Or at least that was the conclusion I reached last time I had the same idea ;-) My thought was more simpleminded, but I don't know this code. Why do we need a cache of the original image once the pixmap has been set? If the original is changed, then presumably we want to reload. But if it doesn't change, we don't need access to it any more, do we? Correct. So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? You can try. Last time I did, I failed. But I was so upset about this piece of code that maybe the solution was much simpler than I thought... Abdel.
Re: Memory leak?
On 08/11/2009 03:40 AM, Abdelrazak Younes wrote: So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? You can try. Last time I did, I failed. But I was so upset about this piece of code that maybe the solution was much simpler than I thought... Hmm. That's a little too brute force. We need the cache item, for monitoring and the like. What we don't need is to have the image itself still there. Does that seem right? (Sorry, I don't know this code and am just poking around.) rh
Re: Memory leak?
On 10/08/2009 21:34, Pavel Sanda wrote: hi, for the third time my system get frozen because of bug http://www.lyx.org/trac/ticket/5002. it seems when we load eg. jpg image to cache and rescale it, the original image is not forgoten so even small scale is of no help and more print-ready image are able to knock system down. the code looks like as we intend to free this memory, but it wont happen: @@ -108,9 +108,12 @@ bool GuiImage::setPixmap(Params const params) // Clear the pixmap to save some memory. if (is_transformed_) original_ = QImage(); this code should work? if i manually add original_.~QImage(); memory is rescued and no swapping happens. however i get some crash in lyx quit then; aparently graphics cache is more intricated stuff... Very intricate! The problem is not coming from Qt but from our own caching mechanism which is heavy and is redundant with Qt own caching system. Last time I tried to clean up this mess I stopped at this issue with a big headache... The only sane solution is to get rid of this complicated caching engine and to rewrite everything with Qt only. Abdel.
Re: Memory leak?
On 10/08/2009 21:39, Abdelrazak Younes wrote: On 10/08/2009 21:34, Pavel Sanda wrote: hi, for the third time my system get frozen because of bug http://www.lyx.org/trac/ticket/5002. it seems when we load eg. jpg image to cache and rescale it, the original image is not forgoten so even small scale is of no help and more print-ready image are able to knock system down. the code looks like as we intend to free this memory, but it wont happen: @@ -108,9 +108,12 @@ bool GuiImage::setPixmap(Params const params) // Clear the pixmap to save some memory. if (is_transformed_) original_ = QImage(); this code should work? if i manually add original_.~QImage(); memory is rescued and no swapping happens. however i get some crash in lyx quit then; aparently graphics cache is more intricated stuff... Very intricate! The problem is not coming from Qt but from our own caching mechanism which is heavy and is redundant with Qt own caching system. Last time I tried to clean up this mess I stopped at this issue with a big headache... The only sane solution is to get rid of this complicated caching engine and to rewrite everything with Qt only. Now that I read my comments in the bug, I see that I reached to the same conclusion one year ago :-) Abdel.
Re: Memory leak?
Abdelrazak Younes wrote: Very intricate! The problem is not coming from Qt but from our own caching mechanism which is heavy and is redundant with Qt own caching system. Last time I tried to clean up this mess I stopped at this issue with a big headache... The only sane solution is to get rid of this complicated caching engine and to rewrite everything with Qt only. but my question is simpler now: why should original_ = QImage(); release the memory as the comment above says? if not what would be correct way of releasing it? pavel
Re: Memory leak?
On 10/08/2009 21:46, Pavel Sanda wrote: Abdelrazak Younes wrote: Very intricate! The problem is not coming from Qt but from our own caching mechanism which is heavy and is redundant with Qt own caching system. Last time I tried to clean up this mess I stopped at this issue with a big headache... The only sane solution is to get rid of this complicated caching engine and to rewrite everything with Qt only. but my question is simpler now: why should original_ = QImage(); release the memory as the comment above says? if not what would be correct way of releasing it? This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. Abdel.
Re: Memory leak?
Abdelrazak Younes wrote: but my question is simpler now: why should original_ = QImage(); release the memory as the comment above says? if not what would be correct way of releasing it? This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) pavel
Re: Memory leak?
On 10/08/2009 21:55, Pavel Sanda wrote: Abdelrazak Younes wrote: but my question is simpler now: why should original_ = QImage(); release the memory as the comment above says? if not what would be correct way of releasing it? This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means Here we clear the pixmap 'original_' to save the memory that it occupies; I don't know to express it in a better way... Abdel.
Re: Memory leak?
Abdelrazak Younes wrote: This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means Here we clear the pixmap 'original_' to save the memory that it occupies; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? pavel
Re: Memory leak?
On 10/08/2009 22:11, Pavel Sanda wrote: Abdelrazak Younes wrote: This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means Here we clear the pixmap 'original_' to save the memory that it occupies; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? Because the original_ pixmap has a shadow copy somewhere else deep inside our graphics system. So deep that it's basically a nightmare to retrieve it at this point. When you call the destructor this shadow copy is of course erased at the same time. This explain why you get a crash at exit. Abdel.
Re: Memory leak?
On 08/10/2009 04:16 PM, Abdelrazak Younes wrote: On 10/08/2009 22:11, Pavel Sanda wrote: Abdelrazak Younes wrote: This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means Here we clear the pixmap 'original_' to save the memory that it occupies; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? Because the original_ pixmap has a shadow copy somewhere else deep inside our graphics system. So deep that it's basically a nightmare to retrieve it at this point. When you call the destructor this shadow copy is of course erased at the same time. This explain why you get a crash at exit. Can you explain what you mean by shadow copy here? Do you mean there is some other variable that points to it, or contains a reference to it, or something of the sort? rh
Re: Memory leak?
On 10/08/2009 22:18, rgheck wrote: On 08/10/2009 04:16 PM, Abdelrazak Younes wrote: On 10/08/2009 22:11, Pavel Sanda wrote: Abdelrazak Younes wrote: This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means Here we clear the pixmap 'original_' to save the memory that it occupies; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? Because the original_ pixmap has a shadow copy somewhere else deep inside our graphics system. So deep that it's basically a nightmare to retrieve it at this point. When you call the destructor this shadow copy is of course erased at the same time. This explain why you get a crash at exit. Can you explain what you mean by shadow copy here? Do you mean there is some other variable that points to it, or contains a reference to it, or something of the sort? QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. Abdel.
Re: Memory leak?
On Mon, Aug 10, 2009 at 09:34:00PM +0200, Pavel Sanda wrote: hi, for the third time my system get frozen because of bug http://www.lyx.org/trac/ticket/5002. it seems when we load eg. jpg image to cache and rescale it, the original image is not forgoten so even small scale is of no help and more print-ready image are able to knock system down. the code looks like as we intend to free this memory, but it wont happen: @@ -108,9 +108,12 @@ bool GuiImage::setPixmap(Params const params) // Clear the pixmap to save some memory. if (is_transformed_) original_ = QImage(); this code should work? Yes. if i manually add original_.~QImage(); memory is rescued and no swapping happens. however i get some crash in lyx quit then; aparently graphics cache is more intricated stuff... Probably because the destructor runs twice: once called explicitly by you and the second time wheneven the object containing original_ is destroyed. Andre'
Re: Memory leak?
Abdelrazak Younes wrote: QPixmap are implicitly shared. this was the missing brick.. thanks pavel
Re: Memory leak?
On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_-image()-clone()); bool const success = image_-setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? Richard
Re: Memory leak?
On 10/08/2009 22:46, rgheck wrote: On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_-image()-clone()); bool const success = image_-setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? We can, if you manage to give access to the original image inside GuiImage::setPixmap(). Without major surgery I am sure this is possible. Or at least that was the conclusion I reached last time I had the same idea ;-) Abdel.
Re: Memory leak?
On 08/10/2009 05:01 PM, Abdelrazak Younes wrote: On 10/08/2009 22:46, rgheck wrote: On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_-image()-clone()); bool const success = image_-setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? We can, if you manage to give access to the original image inside GuiImage::setPixmap(). Without major surgery I am sure this is possible. Or at least that was the conclusion I reached last time I had the same idea ;-) My thought was more simpleminded, but I don't know this code. Why do we need a cache of the original image once the pixmap has been set? If the original is changed, then presumably we want to reload. But if it doesn't change, we don't need access to it any more, do we? So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? rh Abdel.
Re: Memory leak?
Richard Heck wrote: My thought was more simpleminded, but I don't know this code. Why do we need a cache of the original image once the pixmap has been set? If the original is changed, then presumably we want to reload. But if it doesn't change, we don't need access to it any more, do we? So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? maybe we want it when user want eg rescale to a different value? pavel
Re: Memory leak?
On 08/10/2009 06:23 PM, Pavel Sanda wrote: Richard Heck wrote: My thought was more simpleminded, but I don't know this code. Why do we need a cache of the original image once the pixmap has been set? If the original is changed, then presumably we want to reload. But if it doesn't change, we don't need access to it any more, do we? So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? maybe we want it when user want eg rescale to a different value? Though then you have it, and so get the memory issues you're seeing. So we have to let it go, I think. In any event, the rescale will happen little enough that we can re-read from the disk then. rh pavel
Re: Memory leak?
On 10/08/2009 21:34, Pavel Sanda wrote: hi, for the third time my system get frozen because of bug http://www.lyx.org/trac/ticket/5002. it seems when we load eg. jpg image to cache and rescale it, the original image is not forgoten so even small scale is of no help and more print-ready image are able to knock system down. the code looks like as we intend to free this memory, but it wont happen: @@ -108,9 +108,12 @@ bool GuiImage::setPixmap(Params const& params) // Clear the pixmap to save some memory. if (is_transformed_) original_ = QImage(); this code should work? if i manually add original_.~QImage(); memory is rescued and no swapping happens. however i get some crash in lyx quit then; aparently graphics cache is more intricated stuff... Very intricate! The problem is not coming from Qt but from our own caching mechanism which is heavy and is redundant with Qt own caching system. Last time I tried to clean up this mess I stopped at this issue with a big headache... The only sane solution is to get rid of this complicated caching engine and to rewrite everything with Qt only. Abdel.
Re: Memory leak?
On 10/08/2009 21:39, Abdelrazak Younes wrote: On 10/08/2009 21:34, Pavel Sanda wrote: hi, for the third time my system get frozen because of bug http://www.lyx.org/trac/ticket/5002. it seems when we load eg. jpg image to cache and rescale it, the original image is not forgoten so even small scale is of no help and more print-ready image are able to knock system down. the code looks like as we intend to free this memory, but it wont happen: @@ -108,9 +108,12 @@ bool GuiImage::setPixmap(Params const& params) // Clear the pixmap to save some memory. if (is_transformed_) original_ = QImage(); this code should work? if i manually add original_.~QImage(); memory is rescued and no swapping happens. however i get some crash in lyx quit then; aparently graphics cache is more intricated stuff... Very intricate! The problem is not coming from Qt but from our own caching mechanism which is heavy and is redundant with Qt own caching system. Last time I tried to clean up this mess I stopped at this issue with a big headache... The only sane solution is to get rid of this complicated caching engine and to rewrite everything with Qt only. Now that I read my comments in the bug, I see that I reached to the same conclusion one year ago :-) Abdel.
Re: Memory leak?
Abdelrazak Younes wrote: > Very intricate! > > The problem is not coming from Qt but from our own caching mechanism which > is heavy and is redundant with Qt own caching system. Last time I tried to > clean up this mess I stopped at this issue with a big headache... The only > sane solution is to get rid of this complicated caching engine and to > rewrite everything with Qt only. but my question is simpler now: why should original_ = QImage(); release the memory as the comment above says? if not what would be correct way of releasing it? pavel
Re: Memory leak?
On 10/08/2009 21:46, Pavel Sanda wrote: Abdelrazak Younes wrote: Very intricate! The problem is not coming from Qt but from our own caching mechanism which is heavy and is redundant with Qt own caching system. Last time I tried to clean up this mess I stopped at this issue with a big headache... The only sane solution is to get rid of this complicated caching engine and to rewrite everything with Qt only. but my question is simpler now: why should original_ = QImage(); release the memory as the comment above says? if not what would be correct way of releasing it? This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. Abdel.
Re: Memory leak?
Abdelrazak Younes wrote: >> but my question is simpler now: >> why should original_ = QImage(); release the memory as the comment above >> says? >> if not what would be correct way of releasing it? >> > > This code works and is not the problem. I don't remember the details but > the original image is also stored somewhere else in the graphics cache, > AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) pavel
Re: Memory leak?
On 10/08/2009 21:55, Pavel Sanda wrote: Abdelrazak Younes wrote: but my question is simpler now: why should original_ = QImage(); release the memory as the comment above says? if not what would be correct way of releasing it? This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means "Here we clear the pixmap 'original_' to save the memory that it occupies"; I don't know to express it in a better way... Abdel.
Re: Memory leak?
Abdelrazak Younes wrote: >>> This code works and is not the problem. I don't remember the details but >>> the original image is also stored somewhere else in the graphics cache, >>> AFAIR. >>> >> >> which doesn't help me to understand what '// Clear the pixmap to save some >> memory' >> means :) >> >> > It means "Here we clear the pixmap 'original_' to save the memory that it > occupies"; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? pavel
Re: Memory leak?
On 10/08/2009 22:11, Pavel Sanda wrote: Abdelrazak Younes wrote: This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means "Here we clear the pixmap 'original_' to save the memory that it occupies"; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? Because the original_ pixmap has a shadow copy somewhere else deep inside our graphics system. So deep that it's basically a nightmare to retrieve it at this point. When you call the destructor this shadow copy is of course erased at the same time. This explain why you get a crash at exit. Abdel.
Re: Memory leak?
On 08/10/2009 04:16 PM, Abdelrazak Younes wrote: On 10/08/2009 22:11, Pavel Sanda wrote: Abdelrazak Younes wrote: This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means "Here we clear the pixmap 'original_' to save the memory that it occupies"; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? Because the original_ pixmap has a shadow copy somewhere else deep inside our graphics system. So deep that it's basically a nightmare to retrieve it at this point. When you call the destructor this shadow copy is of course erased at the same time. This explain why you get a crash at exit. Can you explain what you mean by "shadow copy" here? Do you mean there is some other variable that points to it, or contains a reference to it, or something of the sort? rh
Re: Memory leak?
On 10/08/2009 22:18, rgheck wrote: On 08/10/2009 04:16 PM, Abdelrazak Younes wrote: On 10/08/2009 22:11, Pavel Sanda wrote: Abdelrazak Younes wrote: This code works and is not the problem. I don't remember the details but the original image is also stored somewhere else in the graphics cache, AFAIR. which doesn't help me to understand what '// Clear the pixmap to save some memory' means :) It means "Here we clear the pixmap 'original_' to save the memory that it occupies"; I don't know to express it in a better way... maybe its just too late for me today and missing something obvious... but why i see huge memory difference for the code if (is_transformed_) { original_.~QImage(); original_ = QImage(); } and for if (is_transformed_) original_ = QImage(); if the second one is intended to clear that pixmap? Because the original_ pixmap has a shadow copy somewhere else deep inside our graphics system. So deep that it's basically a nightmare to retrieve it at this point. When you call the destructor this shadow copy is of course erased at the same time. This explain why you get a crash at exit. Can you explain what you mean by "shadow copy" here? Do you mean there is some other variable that points to it, or contains a reference to it, or something of the sort? QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. Abdel.
Re: Memory leak?
On Mon, Aug 10, 2009 at 09:34:00PM +0200, Pavel Sanda wrote: > hi, > for the third time my system get frozen because of > bug http://www.lyx.org/trac/ticket/5002. > > it seems when we load eg. jpg image to cache and rescale it, the > original image is not forgoten so even small scale is of no help > and more print-ready image are able to knock system down. > > the code looks like as we intend to free this memory, but it > wont happen: > > @@ -108,9 +108,12 @@ bool GuiImage::setPixmap(Params const & params) > // Clear the pixmap to save some memory. > if (is_transformed_) > original_ = QImage(); > > this code should work? Yes. > if i manually add original_.~QImage(); memory is rescued and no > swapping happens. however i get some crash in lyx quit then; aparently > graphics cache is more intricated stuff... Probably because the destructor runs twice: once called explicitly by you and the second time wheneven the object containing original_ is destroyed. Andre'
Re: Memory leak?
Abdelrazak Younes wrote: > QPixmap are implicitly shared. this was the missing brick.. thanks pavel
Re: Memory leak?
On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_->image()->clone()); bool const success = image_->setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? Richard
Re: Memory leak?
On 10/08/2009 22:46, rgheck wrote: On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_->image()->clone()); bool const success = image_->setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? We can, if you manage to give access to the original image inside GuiImage::setPixmap(). Without major surgery I am sure this is possible. Or at least that was the conclusion I reached last time I had the same idea ;-) Abdel.
Re: Memory leak?
On 08/10/2009 05:01 PM, Abdelrazak Younes wrote: On 10/08/2009 22:46, rgheck wrote: On 08/10/2009 04:21 PM, Abdelrazak Younes wrote: QPixmap are implicitly shared. So this means that when you do a = b, a will be a shadow copy of b up until it is modified, thus occupying zero additional memory. This is the same concept as shared pointer, the object, and thus the memory, is not cleared up until the last use of it is destroyed. OK, so here's an idea. The problem lies more or less in Loader::Impl::createPixmap(), here: image_.reset(cached_item_->image()->clone()); bool const success = image_->setPixmap(params_); The first line clones the original image, if I'm not mistaken, and all the transformations then happen in the call to GuiImage::setPixmap(). But the cached original remains. So can we clear that once this has happened? We can, if you manage to give access to the original image inside GuiImage::setPixmap(). Without major surgery I am sure this is possible. Or at least that was the conclusion I reached last time I had the same idea ;-) My thought was more simpleminded, but I don't know this code. Why do we need a cache of the original image once the pixmap has been set? If the original is changed, then presumably we want to reload. But if it doesn't change, we don't need access to it any more, do we? So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? rh Abdel.
Re: Memory leak?
Richard Heck wrote: > My thought was more simpleminded, but I don't know this code. Why do we > need a cache of the original image once the pixmap has been set? If the > original is changed, then presumably we want to reload. But if it doesn't > change, we don't need access to it any more, do we? So, if setPixmap() > returns true, then can't we do something like reset() the cached_item_ and > just get rid if that QImage? maybe we want it when user want eg rescale to a different value? pavel
Re: Memory leak?
On 08/10/2009 06:23 PM, Pavel Sanda wrote: Richard Heck wrote: My thought was more simpleminded, but I don't know this code. Why do we need a cache of the original image once the pixmap has been set? If the original is changed, then presumably we want to reload. But if it doesn't change, we don't need access to it any more, do we? So, if setPixmap() returns true, then can't we do something like reset() the cached_item_ and just get rid if that QImage? maybe we want it when user want eg rescale to a different value? Though then you have it, and so get the memory issues you're seeing. So we have to let it go, I think. In any event, the rescale will happen little enough that we can re-read from the disk then. rh pavel
Re: memory leak in Qt?
Andre Poenitz wrote: On Mon, Dec 03, 2007 at 11:00:00PM +0100, Peter Kümmel wrote: Playing around with vld I came to the conclusion that there is a memory leak in Qt's plugin system. When I trace the allocations of Qt (see ForceIncludeModules in the vld.ini file) I get attached output after just opening and closing lyx. It traces down to the macro in src/corelib/plugin/qplugin.h (macro from 4.4, similar in 4.3.2): #define Q_PLUGIN_INSTANCE(IMPLEMENTATION) \ { \ static QT_PREPEND_NAMESPACE(QPointer)QT_PREPEND_NAMESPACE(QObject) _instance; \ if (!_instance) \ _instance = new IMPLEMENTATION; \ return _instance; \ } Which plugin is this? One of the image plugins. Q_PLUGIN_INSTANCE is documented not to delete the instance, so this seems to 'unlucky, but documented behaviour'... Any reason not to delete it? -- Peter Kümmel
Re: memory leak in Qt?
On Sun, Dec 09, 2007 at 01:25:35PM +0100, Peter Kümmel wrote: #define Q_PLUGIN_INSTANCE(IMPLEMENTATION) \ { \ static QT_PREPEND_NAMESPACE(QPointer)QT_PREPEND_NAMESPACE(QObject) _instance; \ if (!_instance) \ _instance = new IMPLEMENTATION; \ return _instance; \ } Which plugin is this? One of the image plugins. Q_PLUGIN_INSTANCE is documented not to delete the instance, so this seems to 'unlucky, but documented behaviour'... Any reason not to delete it? None that I am aware of. Andre'
Re: memory leak in Qt?
Andre Poenitz wrote: On Mon, Dec 03, 2007 at 11:00:00PM +0100, Peter Kümmel wrote: Playing around with vld I came to the conclusion that there is a memory leak in Qt's plugin system. When I trace the allocations of Qt (see ForceIncludeModules in the vld.ini file) I get attached output after just opening and closing lyx. It traces down to the macro in src/corelib/plugin/qplugin.h (macro from 4.4, similar in 4.3.2): #define Q_PLUGIN_INSTANCE(IMPLEMENTATION) \ { \ static QT_PREPEND_NAMESPACE(QPointer)_instance; \ if (!_instance) \ _instance = new IMPLEMENTATION; \ return _instance; \ } Which plugin is this? One of the image plugins. Q_PLUGIN_INSTANCE is documented not to delete the instance, so this seems to 'unlucky, but documented behaviour'... Any reason not to delete it? -- Peter Kümmel
Re: memory leak in Qt?
On Sun, Dec 09, 2007 at 01:25:35PM +0100, Peter Kümmel wrote: >>> #define Q_PLUGIN_INSTANCE(IMPLEMENTATION) \ >>> { \ >>> static >>> QT_PREPEND_NAMESPACE(QPointer)_instance; >>> \ >>> if (!_instance) \ >>> _instance = new IMPLEMENTATION; \ >>> return _instance; \ >>> } >> Which plugin is this? > > One of the image plugins. > >> Q_PLUGIN_INSTANCE is documented not to delete the instance, so this seems >> to 'unlucky, but documented behaviour'... > > Any reason not to delete it? None that I am aware of. Andre'
Re: memory leak in Qt?
On Mon, Dec 03, 2007 at 11:00:00PM +0100, Peter Kümmel wrote: Playing around with vld I came to the conclusion that there is a memory leak in Qt's plugin system. When I trace the allocations of Qt (see ForceIncludeModules in the vld.ini file) I get attached output after just opening and closing lyx. It traces down to the macro in src/corelib/plugin/qplugin.h (macro from 4.4, similar in 4.3.2): #define Q_PLUGIN_INSTANCE(IMPLEMENTATION) \ { \ static QT_PREPEND_NAMESPACE(QPointer)QT_PREPEND_NAMESPACE(QObject) _instance; \ if (!_instance) \ _instance = new IMPLEMENTATION; \ return _instance; \ } Which plugin is this? Q_PLUGIN_INSTANCE is documented not to delete the instance, so this seems to 'unlucky, but documented behaviour'... Andre'
Re: memory leak in Qt?
On Mon, Dec 03, 2007 at 11:00:00PM +0100, Peter Kümmel wrote: > Playing around with vld I came to the conclusion > that there is a memory leak in Qt's plugin system. > > When I trace the allocations of Qt > (see ForceIncludeModules in the vld.ini file) > I get attached output after just opening and closing lyx. > > > It traces down to the macro in src/corelib/plugin/qplugin.h > (macro from 4.4, similar in 4.3.2): > > #define Q_PLUGIN_INSTANCE(IMPLEMENTATION) \ > { \ > static > QT_PREPEND_NAMESPACE(QPointer)_instance; \ > if (!_instance) \ > _instance = new IMPLEMENTATION; \ > return _instance; \ > } Which plugin is this? Q_PLUGIN_INSTANCE is documented not to delete the instance, so this seems to 'unlucky, but documented behaviour'... Andre'
Re: memory leak in Systemcalls::fork() ?
On Friday 09 March 2001 18:20, Lars Gullik Bjønnes wrote: Angus Leeming [EMAIL PROTECTED] writes: | On Friday 09 March 2001 17:04, Angus Leeming wrote: | and there's more. If I save a file with an external inset, close it and then | open it again, I get the message: | | Solitary \end_inset. Missing \begin_inset?. | Last inset read was: External the reading of external insets does not check for \end_inset and should also remove it form the input stream. this is a bug then. I have fixed this last problem. I'm going to commit John's patch because the problems I have reported in this thread exist in the original code as well. Ie, John has not created them with his patch. Angus
Re: memory leak in Systemcalls::fork() ?
On Friday 09 March 2001 18:20, Lars Gullik Bjønnes wrote: > Angus Leeming <[EMAIL PROTECTED]> writes: > > | On Friday 09 March 2001 17:04, Angus Leeming wrote: > | and there's more. If I save a file with an external inset, close it and then > | open it again, I get the message: > | > | Solitary \end_inset. Missing \begin_inset?. > | Last inset read was: External > the reading of external insets does not check for \end_inset and > should also remove it form the input stream. > > this is a bug then. I have fixed this last problem. I'm going to commit John's patch because the problems I have reported in this thread exist in the original code as well. Ie, John has not created them with his patch. Angus
Re: memory leak in Systemcalls::fork() ?
On Friday 09 March 2001 16:48, Angus Leeming wrote: When "View Result" of the Date command in the external inset popup, I get an error message LyX: execvp failed: No such file or directory and an xterm pops up displaying the message xterm: Can't execvp less aleem@pneumon:PROPOSALS- Back to the problem in hand. Trying to "View Result", LyX executes the command: LyX: execvp failed: No such file or directory python /usr/users/aleem/OTHERS_CODE/lyx/devel/lib//scripts/general_command_wrapper.py - /tmp/lyx_tmpdir4992aaeCaa/lyxext4992aaeCaa date It fails because I don't have python installed on this box. Surely, things should fail gracefully rather than 1. Pop up an xterm and prevent me closing the FormExternal dialog until this xterm is first killed. 2. When I then try and exit LyX, I can't. Instead, I get repeated attempts to popup the xterm. Sometimes, LyX will eventually exit with a core dump, other times I have to "kill" it. Not happy, Angus
Re: memory leak in Systemcalls::fork() ?
On Friday 09 March 2001 17:04, Angus Leeming wrote: and there's more. If I save a file with an external inset, close it and then open it again, I get the message: Solitary \end_inset. Missing \begin_inset?. Last inset read was: External While I'm at it, here's the backtrace from trying to View the Date: A (dbx) r Solitary \end_inset. Missing \begin_inset?. Last inset read was: External New child attached. Use switch to gain access to process 9218 (dbx) switch (dbx) switch 9218 Process 9218: stopped at *[fork__11SystemcallsXv, 0x120376658] ldq r1, 48(sp) (dbx) where 0 fork__11SystemcallsXv(0x1203905b0, 0x1, 0x120390e18, 0x0, 0x120391840) [0x120376658] 1 startscript__11SystemcallsXv(0x1203769d4, 0x113e8, 0x140077ab8, 0x113e8, 0x1203905b0) [0x120376268] 2 startscript__11SystemcallsX9StarttypeRCQ13std60basic_string__TcQ13std15char_traits__TcQ13std13allocator__TvPXQ13std60basic_string__TcQ13std15char_traits__TcQ13std13allocator__Tvi_v(0x120376004, 0x1202ce758, 0x1200d3b20, 0x10002, 0x0) [0x120376a58] 3 executeCommand__C13InsetExternalXRCQ13std60basic_string__TcQ13std15char_traits__TcQ13std13allocator__TvPC6Buffer(0x1400bb8f0, 0x1401add00, 0x14007d300, 0x11490, 0x1400f5648) [0x1202ce754] 4 updateExternal__C13InsetExternalXv(0x0, 0x1d2245ed500, 0x1401ad140, 0x140170e20, 0x140167a90) [0x1202cef34] 5 viewExternal__C13InsetExternalXv(0x12031f7f8, 0x140098520, 0x1401c1f80, 0x0, 0x140170e20) [0x1202cef94] 6 viewCB__12FormExternalXP7flobjs_l(0x1401c1f80, 0x0, 0x140170e20, 0x0, 0x12031daac) [0x12031f7f4] 7 ExternalViewCB(0x140170e20, 0x0, 0x12031daac, 0x1200d8510, 0x3ffbff3e87c) [0x12031daa8] 8 fl_object_qread(0x12031daac, 0x1200d8510, 0x3ffbff3e87c, 0x0, 0x3ffbff538a8) [0x3ffbff3e878] 9 fl_check_forms(0x3ffbff3e87c, 0x0, 0x3ffbff538a8, 0x0, 0x12030511c) [0x3ffbff538a4] 10 runTime__10GUIRunTimeXv(0x14012eb00, 0x0, 0x14012eb60, 0x2bd02000, 0x1401206a0) [0x120305118] 11 runTime__6LyXGUIXv(0x14012eb60, 0x2bd02000, 0x1401206a0, 0x140077a00, 0x1201fc770) [0x1201f895c] 12 __ct__3LyXXPiPPc(0x140065820, 0x1200de150, 0x1201e22ac, 0x140027148, 0x11788) [0x1201fc76c] 13 main(0x0, 0x80084600, 0x1400b2b60, 0x3ff0001, 0x11808) [0x120226e9c] (dbx) c (dbx) c LyX: execvp failed: No such file or directory python /usr/users/aleem/OTHERS_CODE/lyx/devel/lib//scripts/general_command_wrapper.py - /tmp/lyx_tmpdir7920aahxqa/lyxext7920aahxqa date New child attached. Use switch to gain access to process 9181 (dbx) switch 9181 error: cannot transition RUNNING process to READY TO RUN (dbx) Process 9181: stopped at *[fork__11SystemcallsXv, 0x120376658] ldq r1, 48(sp) (dbx) where 0 fork__11SystemcallsXv(0x1203905b0, 0x1, 0x120390e18, 0x0, 0x120391840) [0x120376658] 1 startscript__11SystemcallsXv(0x1203769d4, 0x11448, 0x140077ab8, 0x11448, 0x1203905b0) [0x120376268] 2 startscript__11SystemcallsX9StarttypeRCQ13std60basic_string__TcQ13std15char_traits__TcQ13std13allocator__TvPXQ13std60basic_string__TcQ13std15char_traits__TcQ13std13allocator__Tvi_v(0x120376004, 0x1202ce758, 0x1200d3b20, 0x10002, 0x0) [0x120376a58] 3 executeCommand__C13InsetExternalXRCQ13std60basic_string__TcQ13std15char_traits__TcQ13std13allocator__TvPC6Buffer(0x1400bb8f0, 0x1d2245ed500, 0x14007d300, 0x114e0, 0x1400f5648) [0x1202ce754] 4 viewExternal__C13InsetExternalXv(0x12031f7f8, 0x140098520, 0x1401c1f00, 0x1401c1fa0, 0x140170e20) [0x1202cefe4] 5 viewCB__12FormExternalXP7flobjs_l(0x1401c1f00, 0x1401c1fa0, 0x140170e20, 0x0, 0x12031daac) [0x12031f7f4] 6 ExternalViewCB(0x140170e20, 0x0, 0x12031daac, 0x1200d8510, 0x3ffbff3e87c) [0x12031daa8] 7 fl_object_qread(0x12031daac, 0x1200d8510, 0x3ffbff3e87c, 0x0, 0x3ffbff538a8) [0x3ffbff3e878] 8 fl_check_forms(0x3ffbff3e87c, 0x0, 0x3ffbff538a8, 0x0, 0x12030511c) [0x3ffbff538a4] 9 runTime__10GUIRunTimeXv(0x14012eb00, 0x0, 0x14012eb60, 0x2bd02000, 0x1401206a0) [0x120305118] 10 runTime__6LyXGUIXv(0x14012eb60, 0x2bd02000, 0x1401206a0, 0x140077a00, 0x1201fc770) [0x1201f895c] 11 __ct__3LyXXPiPPc(0x140065820, 0x1200de150, 0x1201e22ac, 0x140027148, 0x11788) [0x1201fc76c] 12 main(0x0, 0x80084600, 0x1400b2b60, 0x3ff0001, 0x11808) [0x120226e9c]
Re: memory leak in Systemcalls::fork() ?
Angus Leeming [EMAIL PROTECTED] writes: | On Friday 09 March 2001 17:04, Angus Leeming wrote: | and there's more. If I save a file with an external inset, close it and then | open it again, I get the message: | | Solitary \end_inset. Missing \begin_inset?. | Last inset read was: External the reading of external insets does not check for \end_inset and should also remove it form the input stream. this is a bug then. Lgb
Re: memory leak in Systemcalls::fork() ?
On Friday 09 March 2001 16:48, Angus Leeming wrote: > When "View Result" of the Date command in the external inset popup, I get an > error message > > LyX: execvp failed: No such file or directory > > and an xterm pops up displaying the message > > xterm: Can't execvp less > aleem@pneumon:PROPOSALS-> Back to the problem in hand. Trying to "View Result", LyX executes the command: LyX: execvp failed: No such file or directory python /usr/users/aleem/OTHERS_CODE/lyx/devel/lib//scripts/general_command_wrapper.py - /tmp/lyx_tmpdir4992aaeCaa/lyxext4992aaeCaa date It fails because I don't have python installed on this box. Surely, things should fail gracefully rather than 1. Pop up an xterm and prevent me closing the FormExternal dialog until this xterm is first killed. 2. When I then try and exit LyX, I can't. Instead, I get repeated attempts to popup the xterm. Sometimes, LyX will eventually exit with a core dump, other times I have to "kill" it. Not happy, Angus