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 Chicken-hackers@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-hackers