fill_params GCI task

[Andrew Whitworth]

I pulled that work into a branch, and I think we should definitely
treat it with serious suspicion. In addition to making major changes
to the form of the code, it very possibly has a performance impact
which we need to benchmark. I think we consider this branch to be
extremely experimental and exploratory, and we don't feel compelled to
merge it unless we think it's a real benefit.

I'm really coming to the conclusion that the problem with fill_params
isn't just that it's too large and ugly, but that it's tasked with too
much stuff. Our PCC algorithm, and the promises that we make about all
the different types and configurations of parameters and arguments is
just too complicated.  For instance:

1) If we add a new PIR op get_signature. Then we can tell IMCC to use
the new op instead of get_params in this situation:

.sub foo
.param pmc sig :call_sig

In these cases, we can call the new op instead of get_params, never
call fill_params, and cut ~10 lines of code out of it. For functions
that use this feature, or HLLs that do their own argument processing
and sig unpacking, this could be a major win.

1a) For that matter, we could replace get_params with a handful of new
ops: get_pmc, get_pmc_slurpy, get_x_named, get_x_optional, etc. Each
one would make a single transaction with the signature object. In this
case, fill_params evaporates entirely, and the onus is put onto the
PIR compiler to generate the argument unpacking code instead of
get_params/fill_params. A huge benefit here is that arguments could be
retrieved *lazily*, only if they are needed in a particular codepath.
What this does do, however, is insert PIR op dispatches between the
processing of individual arguments (which is bad for our current naive
runcores), but each op becomes extremely simple compared to current
fill_params.

2) We know at the very beginning of fill_params whether we need to do
error checking or not. We can break fill_params up into two variants:
One that does no error checking at all, which does not need to check
err_check in tight loops, and one that does full error checking.
Looking at the algorithm, I think we can define the later in terms of
the former, by doing some input checking first, then do err_check-less
fill_params, followed by output checking. This will reduce the size of
all functions involved *and* will be a small performance improvement
for the common case.

3) If we did not do automatic spill-over between named and positional
parameters, we could easily cut fill_params in half.
fill_params_positional would only need to be called if the signature
contains positionals, and fill_params_named would only need to be
called if the signature contains named elements. This would cut
fill_params in half, and maybe be an optimization for the common case
where only positional or only named parameters were used. I realize
that this represents the most major behavior change of all my
suggestions, but I do believe that in the long run the performance
benefits of not checking for parameter spill-over would be worth it.

Simply keeping fill_params as it is now, but breaking it up into
smaller chunks may have a small readability/maintainability benefit if
done correctly (I don't judge whether or not it was done so by the GCI
task), but real benefits are had by changing the algorithm itself. I
think we can easily apply changes like (1) and (2) above. (1a) is more
exploratory, but should be transparent to an HLL. (3) obviously
requires a deprecation cycle.

Thanks,

--Andrew Whitworth



On Fri, Jan 7, 2011 at 7:15 AM, Vasily Chekalkin <bacek at bacek.com> wrote:
> Hello.
>
> I tend to agree with Nick.
>
> --
> Bacek
>
> On Fri, Jan 7, 2011 at 11:04 PM, Nick Wellnhofer <wellnhofer at aevum.de> wrote:
>>
>> I've already mentioned on IRC that I'm skeptical about the GCI task to
>> remove 100 lines from the fill_params function. The task has now been
>> completed and the result looks like this:
>>
>> https://github.com/rofflwaffls/parrot/commit/18d905aec2c0c352c42dd98b7dcf1a0640270929
>>
>> Now we have 270 lines added and some static functions with up to 9
>> arguments. fill_params still has 300 lines of code. In my opinion this is a
>> lot uglier than a 400 lines function.
>>
>> Nick
>> _______________________________________________
>> http://lists.parrot.org/mailman/listinfo/parrot-dev
>>
> _______________________________________________
> http://lists.parrot.org/mailman/listinfo/parrot-dev
>