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
-~----------~----~----~----~------~----~------~--~---
