On 11/30/2018 08:26 AM, Nick Clifton wrote:
> Hi Pedro, Hi Tom,
>
>> Pedro> E.g., in GDB, loading big binaries, demangling is very high
>> Pedro> in profiles, and so we've kicked around the desire to parallelize
>> Pedro> it
>
> I did consider this, but I encountered two problems:
>
> 1. I wanted users of the demangling functions to be able to change
> the recursion limit. So for example in environments with a very
> limited amount of stack space the limit could be reduced. This
> is one of the purposes of the cplus_demangle_set_recursion_limit()
> function.
>
> Since a new d_info structure is created every time a string is demangled
> the only way to implement cplus_demangle_set_recursion_limit would
> be to have it set the value that is used to initialize the field in
> the structure, which is basically back to where we are now.
That's a lesser evil. With the proposed cplus_demangle_set_recursion_limit
interface, you essentially end up with an interface that makes all threads in
the process end up with the same limit. There's precedent for global options
in the current_demangling_style global, set by the cplus_demangle_set_style
function. That's one I recalled, there may be others. I'd prefer interfaces
that pass down the options as arguments, or a pointer to an options struct, but
that's not the main issue.
The main issue is the current recursion level counter.
That's the static variable that I had pointed at:
d_function_type (struct d_info *di)
{
[...]
+ static unsigned long recursion_level = 0;
[...]
+ recursion_level ++;
[...]
+ recursion_level --;
return ret;
}
Even if we go with a recursion limit per process, this recursion
level count must be moved to d_info for thread-safety without
synchronization.
>
> 2. I wanted to be able to access the recursion limit from code
> in cplus-dem.c, which does not use the d_info structure.
That should just mean a bit of plumbing is in order to pass down
either a d_info structure or something like it?
> This was
> the other purpose of the cplus_demangler_set_recursion_limit()
> function - it allows the limit to be read without changing it.
>
> As an alternative I did consider adding an extra parameter to the
> cplus_demangle(), cplus_demangle_opname() and related functions. But
> this would make them non-backwards compatible and I did not want to
> do that.
I'd say that ideally, we'd aim at the interface we want, and then go
from there. If changing interfaces is a problem, we can always add
a new entry point and keep the old as backward compatibility.
But is it (a problem)? Do we require ABI compatibility here?
But again, the main issue I'm seeing is the recursion level counter,
not the global limit.
> I would imagine that changing the recursion limit would be a very
> rare event, possibly only ever done once in an executable's runtime,
> so I doubt that there will ever be any real-life thread safety
> problems associated with it. But I do understand that this is just
> an assumption, not a guarantee.
See above.
Thanks,
Pedro Alves
