git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: 胡哲宁 <adlternative@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "阿德烈 via GitGitGadget" <gitgitgadget@gmail.com>,
	"Git List" <git@vger.kernel.org>
Subject: Re: [PATCH v2] alloc.h|c: migrate alloc_states to mem-pool
Date: Tue, 2 Feb 2021 21:06:17 +0800	[thread overview]
Message-ID: <CAOLTT8ShJzJangqkk8KQVdptFB5K0TiB1ETQcXb=HwinbkL9Hw@mail.gmail.com> (raw)
In-Reply-To: <xmqqpn1jn033.fsf@gitster.c.googlers.com>

To Junio:
Thanks for checking.forget my unprofessional
description.Macroscopically speaking,
both alloc_state and mem_pool are doing one thing:Apply for a
large block of memory in advance,and when needed a dynamically
allocated memory ,we call the interface function to apply for memory,
This can reduce the overhead of calling malloc multiple times.And the
mem-pool or alloc_state will Automatic Expand capacity.

So that ,my this patch may have something not considered,
>     mem_pool_init(o->blob_pool,0);
may be a wrong way to init this mem-pool
because:
>void mem_pool_init(...)
>       ...
>       if (initial_size > 0)
>              mem_pool_alloc_block(pool, initial_size, NULL);
the first time calloc malloc may  decay to we first call "mem_pool_alloc_block",
I think this may not be great.

A little ashamed,I did not consider the optimization point of this
at the beginning,


Junio C Hamano <gitster@pobox.com> 于2021年2月2日周二 上午1:56写道:
>
> "阿德烈 via GitGitGadget"  <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > "alloc_state" may have similar effects with "mem_pool".
>
> What "similar effects" do you have in mind?  "mem_pool" may have
> more than one "effects" to multiple things that are affected, but it
> is unclear which effect that "mem_pool" exerts on what you are
> referring to.
>
> > Using the new memory pool API may be more beneficial
> > to our memory management in the future.
>
> Many things may or may not be "beneficial" in the future.  We do not
> build things on a vague "hunch".
>
Now,I make a rough comparison.
Situation : when we just use it to malloc little struct node
such as `object`,`blob` ,`tree`.
> Are you seeking performance (e.g.  number of objects that can be
> allocated per minute)?

1.  performance.
`mem_pool` api will allocate 2^20 byte everytime ,
`alloc_state` api will allocate 1024*nodesize byte and ALLOC_GROW everytime.
A repo like git may call malloc fewer times when using `mem_pool`,
while a small repo may not have this amount of objects. The number of
calling `malloc`
may be similar.
may be `mem_pool` win a little...
>Are you seeking better memory locality
> (e.g. related objects are likely to be stored in the same page,
> reducing number of page faults)?
2. page faults .
I might think they are similar at first.But now,I start to understand
what you mean:`alloc_state` more like an object pool,so that we could
go through the list of all objects.Therefore, mem_pool is not conducive
to continuous access to all objects.Because There may be fragments
in the memory And this must be a cross-page operation.
so `alloc_state` win.
>Are you seeking reduced wasted
> memory (e.g. custom allocator packs objects better than bog-standard
> malloc(3))?
3.Memory utilization.
`alloc_state`win.No doubt.
 Are you seeking functionality (e.g. you have this and
> that specific codepaths and usecase where you wish to be able to
> release all the objects instantiated for a particular repository,
> without having to go through the list of all objects, and use of
> mempool is one way to allow us do so)?
>
4.functionality
yeah,As mentioned above.Object pool will be better.
`alloc_state`win.
5.
Indeed, the object pool `alloc_state` may be better than the
memory pool `mem_pool`.
But We can assume that the original author’s intention may be to
The five alloc_states are merged together.
Because the original author said: "migrate alloc_states to mem-pool"
Or another advantage of using the memory pool is that it can dynamically
allocate a variety of different objects, I now think the original author has
this intention.So my patch code also needs some modifications.But at the
same time, it may not be good to count them separately if multiple objects
are allocated using the memory pool at the same time.
so 'mem_pool' win a little.
> It is not even clear in your problem description what kind of
> benefit you are seeking, let alone how much quantitative improvement
> you are getting with this change.
>
I don't know how to quantify them temporarily.
I may need the opinions of you and the original author before I can move on.

Thanks.

  reply	other threads:[~2021-02-02 13:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-30 12:59 [PATCH] alloc.h|c: migrate alloc_states to mem-pool 阿德烈 via GitGitGadget
2021-02-01 10:39 ` [PATCH v2] " 阿德烈 via GitGitGadget
2021-02-01 17:55   ` René Scharfe
2021-02-02 13:12     ` 胡哲宁
2021-02-02 16:36       ` René Scharfe.
2021-02-01 17:56   ` Junio C Hamano
2021-02-02 13:06     ` 胡哲宁 [this message]
2021-02-04 10:33   ` [PATCH v3] " 阿德烈 via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOLTT8ShJzJangqkk8KQVdptFB5K0TiB1ETQcXb=HwinbkL9Hw@mail.gmail.com' \
    --to=adlternative@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).