[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or
From: |
Jörg F . Wittenberger |
Subject: |
Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid? |
Date: |
Mon, 29 Feb 2016 11:50:40 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux armv7l; rv:38.0) Gecko/20100101 Icedove/38.4.0 |
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
>
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, (continued)
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Peter Bex, 2016/02/26
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/27
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/27
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/27
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/27
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/28
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/28
- [Chicken-hackers] Saver a patch: Re: Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/28
- Re: [Chicken-hackers] Saver a patch: Re: Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Peter Bex, 2016/02/28
- Re: [Chicken-hackers] Saver a patch: Re: Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Jörg F . Wittenberger, 2016/02/29
- Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?,
Jörg F . Wittenberger <=
Re: [Chicken-hackers] Release blocker? Stack checks completely FUBAR? Or am I just paranoid?, Peter Bex, 2016/02/29