I've added these changes and they'll be part of 0.9.1.0.
- Doug
On Mon, Dec 8, 2008 at 4:39 AM, Phoenix <[EMAIL PROTECTED]> wrote:
>
> #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.11and
> > > > > > > 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
-~----------~----~----~----~------~----~------~--~---
