Re: [PATCH] experimental lookupcar based coverage testing.

[Han-Wen Nienhuys]

Ludovic Courtès escreveu:
>> See attached patch. This still has rough edges. For some reason, I
>> don't catch the memoization of display to #<proc: display>.
> 
> Overall, as Kevin suggested, I'd be more in favor of using the existing
> trap mechanism (possibly extending it if it doesn't provide enough
> information to trap handlers).  However, as you already said, the trap
> mechanism is damn slow.  I guess it is mostly slow because the evaluator
> is slow, but the trap mechanism itself may be optimizable, too.
> 
> If you look at `ENTER_APPLY' around line 3025, it makes at least two
> function calls: `scm_make_debugobj ()' and `scm_call_3 ()'.  The former
> is a one-line function and should really be inlined.  The latter
> introduces unnecessary overhead since it ends up calling `SCM_APPLY ()'
> which in turns necessarily jumps to the `scm_tcs_closures' case since
> trap handlers are always closures.  Thus, at the very least,
> `scm_call_3 ()' should be replaced by `SCM_APPLY ()'.
> 
> These small optimizations would certainly be worthwhile, although
> perhaps not sufficient.

I have doubts whether this can ever be good enough. For effective
coverage analysis, you have a to run an entire test-suite with
coverage enabled.  Eg. for lilypond, the entire test-suite takes 5
minutes on a 1.6ghz Core duo (single thread), when running
normally. That is a lot of Scheme code, and if for every frame-enter
or apply, a piece of user code is called, that will be an enormous
slowdown.

The real problem is not setting up the trap for calling, but rather 
the fact that it 

 - is called for every evaluation (for coverage, it needs to be done
only once)

 - is user-code, ie. something as simple as

  (car x)

(which is just a couple of instructions) will expand into

   (begin
    (vector-set! 
     (hash-ref coverage-table
      (source-property (frame-source (last-frame continuation))
       'filename))
     (source-property (frame-source (last-frame continuation)) 'line)
     #t)
    (car x))

which would likely be a couple of orders of magnitude slower.
  
Of course, the patch that I posted is ad-hoc, because it hardcodes the
coverage analysis in eval.c.  If it were to be included, I propose
something like

 (trap-set! 'memoize-symbol
            record-coverage)
 (trap-enable 'memoize-symbol)

which would be possible with a generic, and quite minimal extension to
eval.

However, I'd like some feedback on the approach before reworking the
ad-hoc patch into a real one.



-- 
 Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen