Hi, Zeid, Thanks for confirming the change. You are right that the function swap is only used in ibis::storage::enlarge. The fix should work correctly.
John On 2/12/2010 8:14 AM, Derhally, Zeid wrote: > That's the change I made in my local sources and that fixes the memory > leak. > > I did not notice the swap method is called from other places, but I did > not have time to investigate whether the reference swap would be needed > or not. > > > -----Original Message----- > From: [email protected] > [mailto:[email protected]] On Behalf Of K. John Wu > Sent: Friday, February 12, 2010 10:03 AM > To: FastBit Users > Subject: Re: [FastBit-users] Memory leak in > fileManager::storage::enlarge ? > > Hi, Zeid, > > Thanks for your helpful explanation. I believe that I have neglected > to take into account of the mismatch between nref=0 and nref=1. There > is a relatively simply fix to the problem, would you mind try it on > you own copy of the copy and let us know whether it fixes the problem. > > The fix is to comment out the last executable line in fileManager.h, > which should look like > > nref.swap(rhs.nref); > > > John > > PS: This is a very small change that you can make to your code without > waiting for the nightly snapshot.. > > > On 2/12/2010 7:03 AM, Derhally, Zeid wrote: >> I am running against multiple data partitions. The current dataset > I'm >> using is around 19 million rows, around 1 GB, and spread cross 6 >> partitions. Unfortunately it contains sensitive data which I can't >> share at this time. >> >> Because the data is so large, I can easily spot the memory usage >> increase around 100 MB with each query run. >> >> When I ran the ibis.exe through the memory profiler it shows that >> m_begin buffer was being leaked. >> >> Stepping through the code, starting on line 1844 in the method > enlarge, >> >> ibis::fileManager::storage cp(nelm); >> memcpy(cp.m_begin, m_begin, oldsize); >> swap(cp); >> >> >> cp's n_ref is zero and the current object's n_ref is 1. The swap > method >> ends up swapping the buffers and the reference. When cp exits scope > and >> is destroyed, it's clear() method will not free m_begin because > nref()> >> 0 >> >> I'm not sure I see a call to nosharing in my stack crawl, which looks >> like >> >> ibis.exe!ibis::fileManager::storage::clear() Line 1863 C++ >> ibis.exe!ibis::fileManager::storage::~storage() Line 270 + 0x45 bytes >> C++ >> ibis.exe!ibis::fileManager::storage::enlarge(unsigned int > nelm=24687688) >> Line 1847 + 0xb bytes C++ >> ibis.exe!ibis::array_t<int>::reserve(unsigned int n=4307553) Line > 1214 >> C++ >> ibis.exe!ibis::util::addIncoreData<int> >> ibis.exe!ibis::table::select >> ibis.exe!ibis::table::select >> ibis.exe!ibis::mensa::select(const char * sel=0x016c0930, const char * >> cond=0x0018f2f0) >> ibis.exe!tableSelect >> ibis.exe!parseString >> ibis.exe!main >> ibis.exe!__tmainCRTStartup() Line 266 + 0x19 bytes C >> ibis.exe!mainCRTStartup() Line 182 C >> >> >> When I get a chance I will try to reproduce it with the test data. >> >> >> -----Original Message----- >> From: [email protected] >> [mailto:[email protected]] On Behalf Of K. John Wu >> Sent: Thursday, February 11, 2010 11:09 PM >> To: FastBit Users >> Subject: Re: [FastBit-users] Memory leak in >> fileManager::storage::enlarge ? >> >> Hi, Zeid, >> >> Thanks for checking this problem out. I am probably missing something >> obvious. Do you have any idea how I might be able to reproduce the >> problem? I thought the variable nref should be one because the called >> to enlarge is always proceeded by a call to nosharing. Please help me >> figure this one out. >> >> John >> >> PS: I presume the problem occurs when querying multiple data >> partitions. If that is the case, you might want to know that 'make >> check' now has some test cases involving multiple data partitions. It >> would be perfect if you can reproduce the problem with data in >> tests/tmp/t2 (after you run make check). Otherwise, you might have to >> give me some sample data to reproduce the problem. >> >> >> On 2/11/2010 8:30 AM, Derhally, Zeid wrote: >>> Hi John, >>> >>> I tried this with 1.1.6 and the memory leak is still present. >>> >>> Looking at the code I still see that the enlarge method is still >> calling >>> the swap method which ends up replacing the reference count of the >>> temporary storage object with the existing one which results in the >>> buffer not getting freed up. >>> >>> Zeid >>> >>> >>> -----Original Message----- >>> From: K. John Wu [mailto:[email protected]] >>> Sent: Wednesday, February 10, 2010 10:41 PM >>> To: FastBit Users >>> Cc: Derhally, Zeid >>> Subject: Re: [FastBit-users] Memory leak in >>> fileManager::storage::enlarge ? >>> >>> Hi, Zeid, >>> >>> Thanks for the report. This particular issue should have been fixed >>> in ibis1.1.6. If you get a chance, please give it a try. >>> >>> There is a note about the change from Jan 29 in ChangeLog, which > reads >>> >>> * src/fileManager.cpp: storage::enlarge used copy-and-swap idiom >>> incorrectly and caused the client to assume the operation was a >>> failure >>> >>> Please feel free to let us know if you continue to see problems with >>> this function. >>> >>> John >>> >>> >>> On 2/10/2010 2:47 PM, Derhally, Zeid wrote: >>>> Hi, >>>> >>>> I believe I've run into a memory leak with ibis 1.1.5 >>>> >>>> In the function, ibis::fileManager::storage::enlarge, there is a > call >>> to >>>> swap() around line 1909, looks like >>>> >>>> try { >>>> >>>> ibis::fileManager::storage cp(nelm); >>>> >>>> memcpy(cp.m_begin, m_begin, oldsize); >>>> >>>> cp.m_end; >>>> >>>> swap(cp); >>>> >>>> } >>>> >>>> catch (...) { >>>> >>>> LOGGER(ibis::gVerbose>= 0) >>>> >>>> << "Warning --"<< evt<< " failed to allocate new storage," >>>> >>>> "current storage unchanged"; >>>> >>>> } >>>> >>>> The swap function also swaps the reference count value which causes >>> the >>>> m_begin buffer to not be freed up because the temporary object on > the >>>> stack inherited the reference count of the current object. >>>> >>>> I've made a change in my sources where I did not swap the reference >>>> count when enlarging the storage and that took care of the leak. >>>> >>>> Thanks. >>>> >>>> Zeid >>>> >>>> >>>> >>>> _______________________________________________ >>>> FastBit-users mailing list >>>> [email protected] >>>> https://hpcrdm.lbl.gov/cgi-bin/mailman/listinfo/fastbit-users >> _______________________________________________ >> FastBit-users mailing list >> [email protected] >> https://hpcrdm.lbl.gov/cgi-bin/mailman/listinfo/fastbit-users >> _______________________________________________ >> FastBit-users mailing list >> [email protected] >> https://hpcrdm.lbl.gov/cgi-bin/mailman/listinfo/fastbit-users > _______________________________________________ > FastBit-users mailing list > [email protected] > https://hpcrdm.lbl.gov/cgi-bin/mailman/listinfo/fastbit-users > _______________________________________________ > FastBit-users mailing list > [email protected] > https://hpcrdm.lbl.gov/cgi-bin/mailman/listinfo/fastbit-users _______________________________________________ FastBit-users mailing list [email protected] https://hpcrdm.lbl.gov/cgi-bin/mailman/listinfo/fastbit-users
