Re: [Libunwind-devel] WIP patch: configurable cache size

[Arun Sharma]

[ Sorry for the late response ]

On Fri, Oct 17, 2014 at 11:38 AM, Milian Wolff <address@hidden> wrote:

>> > * Does it make sense to add a new file or just add it to the existing
>> > *caching_policy.c?
>>
>> That's what I deduced from the existence of *caching_policy.c with just one
>> function (and the file is named after the function). I'm also fine with
>> adding the methods there if that's OK for you. I'll do that in the
>> follow-up commit then
>
> I've merged this locally now.
>

Thanks.

>> > * global vs thread local vs none should be based on the caching
>> > policy. It's possible that this is what you've done.
>>
>> That is actually something I'm not so sure about I did correctly. If I'm not
>> mistaken, the old cache behavior was always global, no? It always used the
>> cache set in the unw_addr_space_t (which is global, and not per-thread,
>> no?). Now, with the code I copied over, I think it's going to be
>> global/thread- local/none as you say. But if someone could review that, I'd
>> appreciate that.

I'll do that in another email.

>>
>> > * s/unthreaded/global/
>>
>> OK, note that most of this code comes from src/x86_64/Gtrace.c which uses
>> the "unthreaded" nomenclature. Generally, is there a way in C to share
>> some/most of this code somehow? In C++, I'd use a template, in C - what do
>> I do? Copy'n'paste the stuff as it's done now, or macrofy more of this and
>> share the code that way?

We have libunwind/src/mi (for machine independent code). If this is not machine
independent code, but merely code reusable in several archs, we could
create another top level directory.

We did something like this for the linux kernel about 10 years ago.
All the archs
were duplicating very similar code/system calls for 32 bit
compatibility. We spent some time moving code into:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/kernel/compat.c

include/asm-generic is a similar concept.

>
> Ping, any input on the two above points?
>
>> > * > unw_set_cache_log_size - why not unw_set_cache_size() and then
>> > round up to the nearest power of 2?
>>
>> Can do if you prefer that.
>
> [...]
> unw_set_cache_log_size(10)
>
> or
>
> unw_set_cache_size(1024)
>

As a user of this API, the latter seems simpler. Things like mmap(2)
do such rounding up (to nearest page size).

 -Arun