Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?

[Jörg F . Wittenberger]

PS: regarding the pre-allocated argvector I forgot to mention one
additional change you need to make: some procedures in runtime.c (e.g.,
values_continuation) either still need to C_alloc or be changed to work
in case av==av2 holds.  Look in the diff for C_memcpy replaced by C_memmove.

Am 28.02.2016 um 15:19 schrieb Jörg F. Wittenberger:
> There is an number of arguments limit documented in "Deviations from the
> standard".  (BTW: I guess the 1000 claimed there is outdated.)
> 
> But is this limit checked anywhere?  My reading of C_apply makes be
> wonder.  Am I missing a check elsewhere?
> 
> Otherwise I'd add it with the patch.
> 
> BTW: currently a make check completed here.  This one re-uses a single,
> stack allocated argvector of TEMPORARY_STACK_SIZE whenever the current
> code would do C_alloc or av2[N].  Notable exception C_context_switch.
> Interestingly: while this is the simplest code one could use, the length
> tagged argvector is consistently about 2% faster here on all tests so far.
> 
> Cheers
> 
> /Jörg
> 
> Am 27.02.2016 um 20:58 schrieb Jörg F. Wittenberger:
>> Am 27.02.2016 um 14:46 schrieb Jörg F. Wittenberger:
>>> Am 27.02.2016 um 13:09 schrieb Jörg F. Wittenberger:
>>>> Am 27.02.2016 um 12:25 schrieb Jörg F. Wittenberger:
>>>>> Hi folks,
>>>>>
>>>>> if you really consider anything to be done to the argvector handling
>>>>> before the next release
>>>>
>>>> ...
>>>>
>>>> I wonder: why not malloc exactly one argvector of TEMPORARY_STACK_SIZE
>>>> word and drop all the checking?
>>>>
>>>> Then either the current av vector is the one passed in, then we can
>>>> safely reuse it.  If it's not we re-use the global anyway.
>>>
>>> Looking at those options I wonder if there is not an even better option.
>>>
>>> This is the relevant change in my patch:
>>>
>>> #if USE_OLD_AV
>>> #define C_allocate_fresh_argvector(n) C_alloc(n)
>>> #define C_allocate_argvector(c, av, avl) ( (c >= avl) ? av :
>>> C_force_allocate_fresh_argvector(avl))
>>> #define C_argvector_flush() /* does nothing */
>>>
>>> Those two are the macros doing the argvector handling.  Here mapping
>>> back to equivalent code of what master would do.  Only those are called
>>> from runtime.c and the C backend.  Well, not exactly, there is a
>>> C_kontinue_av, which is a av C_kontinue. Trivial.  However runtime.c
>>> already uses it (this is where the performance gain came in).  But it's
>>> #defined back to C_kontinue depending on USE_OLD_AV anyways.
>>>
>>> #else
>>>
>>> This is the implementation of the length tagged argvector.
>>>
>>> #define C_argvector_reuse_dflt(n)  ((C_default_argvector_value != NULL)
>>> && (C_default_argvector_value[0] >= (n)))
>>> #define C_argvector_flush()        (C_default_argvector_value = NULL)
>>>
>>> This would have to be changed to possibly reset the
>>> C_default_argvector_value and return the temporary_stack.
>>>
>>> #define C_force_allocate_fresh_argvector(n) ((C_default_argvector_value
>>> = C_alloc((n)+1)), *C_default_argvector_value=(n),
>>> C_default_argvector_value+1)
>>>
>>> #define C_allocate_fresh_argvector(avl) (C_argvector_reuse_dflt(avl) ?
>>> C_default_argvector_value+1 : C_force_allocate_fresh_argvector(avl))
>>> #define C_argvector_size(av) (av[-1])
>>> #define C_allocate_argvector(c, av, avl) ((((c) >= (avl)) ||
>>> (C_argvector_size(av) >= (avl))) ? av :
>>> C_force_allocate_fresh_argvector(avl))
>>> #endif
>>>
>>> If, instead, we would only ever put a pointer to a stack allocated
>>> argument vector large enough for the apply count into the
>>> C_default_argvector_value.  Then we could forgo the whole length tagging
>>> and checking.  Even against "c" we may or may not want to check
>>> (profiling will show).
>>>
>>> Something like this should work:
>>>
>>> #define C_allocate_fresh_argvector(avl) ( C_default_argvector_value !=
>>> NULL ? C_default_argvector_value : C_demand(avl) ?
>>> C_default_argvector_value = C_alloc(TEMPORARY_STACK_SIZE) :
>>> C_default_argvector_value = NULL, temporary_stack)
>>>
>>> Except for the TEMPORARY_STACK_SIZE, which is not available to macros
>>> and that's a good thing.  However as this not in no way critical code,
>>> we might want to call into runtime.c like
>>> C_a_i_allocate_fresh_argvector(naming convections?) which would respect
>>> runtime options etc.
>>
>>
>> "make check" just passed for modifications like this
>>
>> #if USE_OLD_AV  // NOT APPLICABLE
>> ...
>> #elsif USE_FIXED_DFLT  // THIS IS WHAT IS ACTUALLY EXPANDED
>> // TBD: runtime internal
>> #define C_argvector_flush()        (C_default_argvector_value = NULL)
>> #define C_argvector_size(av) (C_default_argvector_value == av ? /*FIXME
>> TEMPORARY_STACK_SIZE should be in runtime.c where it is known */ 4096 : 0)
>>
>> #define C_force_allocate_fresh_argvector(avl) ( C_demand(avl) ?
>> (C_default_argvector_value = C_alloc(avl)) : (C_argvector_flush(),
>> C_temporary_stack))
>>
>> // TDB: leave only these exported #defines
>> #define C_allocate_fresh_argvector(avl) ( /*FIXME should we
>> assert(avl<=limit)*/ C_default_argvector_value != NULL ?
>> C_default_argvector_value : C_force_allocate_fresh_argvector(avl))
>>
>> // TBD: try this, may be faster: #define C_allocate_argvector(c, av,
>> avl) ( (c >= avl) ? av : C_force_allocate_fresh_argvector(avl))
>> // TBD: try assert(C_default_argvector_value != NULL) here and never
>> look back if this works. (deferred)
>> #define C_allocate_argvector(c, av, avl) ( (C_default_argvector_value !=
>> NULL) ? C_default_argvector_value : C_force_allocate_fresh_argvector(avl))
>>
>> #else
>> ... the length tagged argvector version here
>> #endif
>>
>>
>> Let me call it a day now.
>>
>> Tomorrow is the dog's day.  More testing, benchmark and a patch not
>> before Monday.
>>
>> Cheers
>>
>> /Jörg
>>
>>
>>>
>>> As long as no code flushes the C_default_argvector value, that one will
>>> be reused anyway.  Flushing it does the C_reclaim.  Last resort is the
>>> temporary stack.  Passing as argument vector we still can anything.
>>>
>>>> The downside: now we have one more area to scan in the garbage
>>>> collector.  (That's why I preferred to stack allocated one so far).
>>>
>>> Upside: not additional code in the gc.
>>>
>>>> However reading the related code I get the feeling that we even could
>>>> simply use the temporary_stack as the argvector.  However I'm not sure
>>>> about that one.
>>>>
>>>> Just thoughts.
>>>
>>> Comments?
> 
> 
> _______________________________________________
> Chicken-hackers mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/chicken-hackers
>