#define WORD_SIZE      (sizeof(void*))

   /* We put data of the the same type together, "is_map" is used for
CellMap */
        void *allocate(size_t size, bool is_map = false) {
                boost::mutex::scoped_lock lock(m_mutex);

                /* make "size" align to the machine word size */
                if (size & (WORD_SIZE - 1)) {
                        size = (size + WORD_SIZE) & ~(WORD_SIZE - 1);
                }

               ...


This should be OK.

And by the way, the function  dump_stat has some defects. If m_cuf_buf
== NULL, the function will fail.
This one should be OK.
        void dump_stat() {
                int i = 0;
                BufNode *p = m_cur_buf;
                while (p) {
                        p = p->m_prev;
                        i++;
                }

                std::cout << "Current Pool Size : " << i * m_buf_size
+ m_head_ptr - m_tail_ptr <<"; Free size : " << m_tail_ptr -
m_head_ptr << std::endl;
        }


On Dec 6, 10:17 am, Luke <[EMAIL PROTECTED]> wrote:
> I'm reviewing the code. There are a few issues:
>
> 1. The current implementation will cause unaligned memory access,
> which is 2x-3x slower than aligned access on x86/x86_64 and will crash
> on IA64 and other architectures.
> 2. The mutex for the pool doesn't seem necessary as the pool is owned
> by the CellCache, which is already protected by its mutex.
>
> __Luke
>
> On Dec 2, 10:48 am, "Doug Judd" <[EMAIL PROTECTED]> wrote:
>
> > HI Phoenix,
>
> > Thanks!  I'll pull it in and set the default buffer size to 512K.
>
> > - Doug
>
> > On Tue, Dec 2, 2008 at 10:40 AM, Phoenix <[EMAIL PROTECTED]> wrote:
>
> > > Hi Doug,
> > > I've uploaded a new patch in the Files section.
>
> > >http://hypertable-dev.googlegroups.com/web/mem-pool-v2.patch?hl=en&gs...
>
> > > This patch indicates the GPL license and this time we give a list
> > > version of memory pool.
>
> > > BTW, we tested the 4KB buffer version of the memory pool, and found
> > > that its performance is very similar to the tc-malloc one. We think
> > > that this is because there are lots of internal fragments when using
> > > 4KB buffer.
> > > And after several tests, we think 512KB is a good trade-off between
> > > the waste of memory and the internal fragments. I will upload a
> > > picture later.
>
> > > Thanks!
>
> > > On Dec 2, 9:12 am, "Doug Judd" <[EMAIL PROTECTED]> wrote:
> > > > Hi Phoenix,
>
> > > > I just merged in this patch and noticed one thing.  In the new files 
> > > > that
> > > > you created, the header comment does not indicate the license.  For me 
> > > > to
> > > > pull this patch into the code base, it needs to be GPL 2.0+.  Can you
> > > > re-create this patch and modify the header comments for CellCachePool.h
> > > and
> > > > CellCachePoolAllocator.h to include the boilerplate GPL language:
>
> > > > ----------- cut -------------
> > > >  * This file is part of
> > > > Hypertable.
>
> > > >  *
>
> > > >  * Hypertable is free software; you can redistribute it
> > > > and/or
>
> > > >  * modify it under the terms of the GNU General Public
> > > > License
>
> > > >  * as published by the Free Software Foundation; either version 2
> > > >  * of the License, or any later version.
> > > >  *
>
> > > >  * Hypertable is distributed in the hope that it will be
> > > > useful,
>
> > > >  * but WITHOUT ANY WARRANTY; without even the implied warranty
> > > > of
>
> > > >  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> > > > the
>
> > > >  * GNU General Public License for more
> > > > details.
>
> > > >  *
>
> > > >  * You should have received a copy of the GNU General Public
> > > > License
>
> > > >  * along with this program; if not, write to the Free
> > > > Software
>
> > > >  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> > > > MA
>
> > > >  * 02110-1301, USA.
> > > > ---------- cut ---------------
>
> > > > Thanks!
>
> > > > - Doug
>
> > > > On Mon, Nov 24, 2008 at 6:11 AM, Phoenix <[EMAIL PROTECTED]>
> > > wrote:
>
> > > > > sorry, the patch above has a little problem, I upload a earlier
> > > > > version. This one
>
> > > > >http://hypertable-dev.googlegroups.com/web/mem-pool.patch?hl=en&gsc=-.
> > > ..
> > > > > is OK.
>
> > > > > On Nov 24, 10:00 pm, Phoenix <[EMAIL PROTECTED]> wrote:
> > > > > > Hi Doug,
> > > > > >   In our using of Hypertable, its memory usage is too large. We
> > > tested
> > > > > > it and found that the major problem laied in the CellCache. The data
> > > > > > below is from the google heap profiler:
>
> > > > > > <Test Environmet: 16GB Mem, Intel(R) Xeon(R) [EMAIL PROTECTED] * 4,
> > > rhel
> > > > > > as4u3>
>
> > > > > >   Function (during
> > > > > > execution)
> > >  Memory
> > > > > > Usage
>
> > > > > > Hypertable::CellCache::add
> > > > > > 75.6%
>
> > > > > > __gnu_cxx::new_allocator::allocate
> > > > > > 18.8%
>
> > > > > > Hypertable::DynamicBuffer::grow
> > > > > > 4.1%
>
> > > > > > Hypertable::IOHandlerData::handle_event
> > > > > > 1.0%
>
> > > > > > Hypertable::BlockCompressionCodecLzo::BlockCompressionCodecLzo
> > > > > > 0.5%
>
> > > > > >   We found that the main problem laid in the CellCache(the second 
> > > > > > one
> > > > > > "allocate" is called by CellMap, which is also in the CellCache). 
> > > > > > And
> > > > > > after a long time of inserting data, the memory usage keeps a very
> > > > > > high level, which we thought should be freed after doing some
> > > > > > compaction work. In our a ten-server cluster, one range(in this case
> > > > > > we set only a  AccessGroup for each table) used about 32MB. And the
> > > > > > memory is never freed.
>
> > > > > >   After we made some tests and experiments, we implemented a memory
> > > > > > pool for CellCache. After about one week's tests, it works well and
> > > > > > effciently. In the some cluster as mentioned above, each range only
> > > > > > use  about 1.2MB on average, after very short time of the completing
> > > > > > of inserting.
>
> > > > > >   We compare it with the standard version in a single server. In the
> > > > > > standard version, whether use tcmalloc or not (tcmalloc can help
> > > some,
> > > > > > it can reduce about 30% of the standard one), the memory usage never
> > > > > > falls down. On contrast, the pool version's memory usage go down
> > > > > > quickly after the inserting is down.
> > > > > >   In the comparation, we insert about 11G data into the hypertable
> > > > > > (about 33 ranges after parsing and inserting). The memory usage in
> > > > > > this process can be seen here <the image and patch is uploaded in 
> > > > > > the
> > > > > > "Files" of this group>
>
> > >http://hypertable-dev.googlegroups.com/web/RS%20Mem-Usage%20Comparati...
> > > > > >   The purple one we use our pool both for <key,value> pairs and the
> > > > > > CellMap; the yellow one is only for the <key, value> pairs. As seen
> > > > > > from this image, the pool version is very excellent in memory usage.
> > > > > >   And the patch's link ishttp://
> > > > > groups.google.com/group/hypertable-dev/web/mem-pool.patch.tgz?...
>
> > > > > >   We use google heap profiler for the pool version and get the
> > > > > > following data:
>
> > > > > > Function (during execution)      Mem  Usage
> > > > > > CellCachePool::get_memory        94.3%
>
> > > > > > Hypertable::
> > > > > > DynamicBuffer::grow              3.8%
> > > > > > Hypertable
> > > > > > ::BlockCompressionCodecLzo
> > > > > > ::BlockCompressionCodecLzo       1.1%
> > > > > > Hypertable
> > > > > > ::IOHandlerData
> > > > > > ::handle_event                   0.5%
>
> > > > > >   BTW, in our tests, the RangeServer crashed when we set
> > > > > > Hypertable.RangeServer.MaintenanceThreads=4 . We test 0.9.0.11 and
> > > > > > 0.9.0.12, both of them have this problem and this week we want to
> > > make
> > > > > > more test about it.
>
> > > > > >   We hope this can help you.
>
> > > > > >   Best wishes.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Hypertable Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/hypertable-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to