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

[Milian Wolff]

Hey Arun,

I'm still waiting for your mail you promised below ;-) Locally, I've rebased 
my patch on current master. I have also some more questions to your comments 
below.

On Thursday 06 November 2014 08:42:33 Arun Sharma wrote:
> [ Sorry for the late response ]
> 
> On Fri, Oct 17, 2014 at 11:38 AM, Milian Wolff <address@hidden> wrote:
> >> > * 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.

You never sent this, did you? Or did I miss this?

> >> > * 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/c
> ompat.c
> 
> include/asm-generic is a similar concept.

What I still don't understand though, is how we could actually share this 
code. I mean, in x86_64/Gtrace.c there's a bunch of code, which is 
semantically "equivalent" to what I added to dwarf/Gparser.c now, but the 
actual types are different. Both represent per-thread/global caches, but one 
uses dwarf_rs_cache and the other unw_trace_cache_t... As I said before, in 
C++ it would be trivial to share this code, but how does one do this in C? 
Macros? Functions that take void*, callbacks, etc.? Or is it OK for me to 
leave this as-is and someone more experienced with C merges the common code 
base?

> > 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).

I'll implement this now.

Bye
-- 
Milian Wolff
address@hidden
http://milianw.de