Yeah, that's a clever hack. I was just a little surprised that
performance didn't really improve after this patch. My benchmark of
the pagearena showed 20% improvement in allocation speed and 10x
speedup in deallocation speed vs malloc/new[]
__Luke
On Dec 10, 5:41 am, Phoenix <[EMAIL PROTECTED]> wrote:
> Hi Doug,
>
> Actually, alignment for CellMap is useless. Because the memory for map
> is allocated from the end of the buffer, it is aligned naturally.
>
> BTW, in dump_stat, "i" should be inited by "0", not "1".
>
> Best wishes!
>
> On Dec 9, 3:58 am, "Doug Judd" <[EMAIL PROTECTED]> wrote:
>
> > 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
-~----------~----~----~----~------~----~------~--~---
