#582 too much consing in IGNORE-ERRORS

closed-fixed
Sam Steingold
clisp (525)
5
2011-03-29
2010-12-30
Sam Steingold
No

in GNU CLISP 2.33.1 (2004-05-22) (and earlier)
(disassemble (LAMBDA (X Y) (IGNORE-ERRORS (= X Y))))
Disassembly of function :LAMBDA
(CONST 0) = NIL
(CONST 1) = (#(ERROR 15) 2 . 1)
2 required arguments
0 optional arguments
No rest parameter
No keyword parameters
16 byte-code instructions:
0 (BLOCK-OPEN 0 L24)
3 (HANDLER-OPEN 1 L15) ; (#(ERROR 15) 2 . 1)
5 (LOAD&PUSH 9)
6 (LOAD&PUSH 9)
7 (CALLSR 1 45) ; =
10 (SKIP 4)
12 (BLOCK-CLOSE)
13 (SKIP&RET 3)
15 L15
15 (HANDLER-BEGIN&PUSH)
16 (NIL&PUSH)
17 (LOAD&PUSH 1)
18 (STACK-TO-MV 2)
20 (RETURN-FROM-I 0 0 2)
24 L24
24 (SKIP&RET 3)

in GNU CLISP 2.34 (2005-07-20) (and later):
(disassemble (LAMBDA (X Y) (IGNORE-ERRORS (= X Y))))
Disassembly of function :LAMBDA
(CONST 0) = NIL
(CONST 1) = #<COMPILED-FUNCTION :LAMBDA-1>
(CONST 2) = #<COMPILED-FUNCTION :LAMBDA-2>
(CONST 3) = (#(ERROR 32) 2 . 1)
2 required arguments
0 optional arguments
No rest parameter
No keyword parameters
25 byte-code instructions:
0 (NIL)
1 (MAKE-VECTOR1&PUSH 2)
3 (LOAD&STOREC 3 0 0)
7 (LOAD&STOREC 2 0 1)
11 (BLOCK-OPEN 0 L47)
14 (LOAD&PUSH 2)
15 (COPY-CLOSURE&PUSH 1 1) ; #<COMPILED-FUNCTION :LAMBDA-1>
18 (LOAD&PUSH 4)
19 (COPY-CLOSURE&PUSH 2 1) ; #<COMPILED-FUNCTION :LAMBDA-2>
22 (HANDLER-OPEN 3 L32) ; (#(ERROR 32) 2 . 1)
24 (LOAD&PUSH 4)
25 (FUNCALL 0)
27 (SKIP 6)
29 (BLOCK-CLOSE)
30 (SKIP&RET 4)
32 L32
32 (HANDLER-BEGIN&PUSH)
33 (LOADI&PUSH 0 0 1)
37 (FUNCALL&PUSH 0)
39 (LOAD&PUSH 1)
40 (FUNCALL 1)
42 (SKIPSP 3 1)
45 (SKIP&RET 2)
47 L47
47 (SKIP&RET 4)

which is longer and conses more.

Discussion

  • Sam Steingold
    Sam Steingold
    2010-12-30

    somehow I cannot compile 2.34 on my laptop (intparam.h:7:2: error: #error "Integers of type int have no binary representation!!" &c) so my investigation is limited to the lisp side.
    looks like older clisp compiles the call to = and values inline which 2.34 creates closures for that (which is kind of crazy).
    apparently caused by my 2005-01-21 patch

     
  • Sam Steingold
    Sam Steingold
    2010-12-30

    • assigned_to: haible --> sds
     
  • Bruno Haible
    Bruno Haible
    2010-12-31

    Yes I agree it's most likely caused by this patch:

    2005-01-21 Sam Steingold <sds@gnu.org>

    * condition.lisp (handler-bind): use the function syntax for
    %HANDLER-BIND
    * compiler.lisp (quote-p): new function
    (l-constantp, c-constantp): use it
    (c-form-table): do not handle HANDLER-BIND specially
    (c-HANDLER-BIND): handle HANDLER-BIND in function syntax
    * init.lisp (%expand-form): do not handle %HANDLER-BIND specially
    * clos-class3.lisp (reinitialize-instance-<defined-class>):
    use use the function syntax for %HANDLER-BIND
    * clos-genfun4.lisp (generic-function-undeterminedp): ditto
    * clos-methcomb2.lisp (any-method-combination-check-options): ditto

    The NEWS entry for this patch was: "Third party code walkers can now handle HANDLER-BIND et al."
    So the objective was to remove the need for code walkers to handle sys::%handler-bind.
    The patch itself was fine. The problem is that this macroexpansion:

    > (macroexpand-1 (third (macroexpand-1 '(IGNORE-ERRORS (= X Y)))))
    (LET
    ((#:G2727
    #'(LAMBDA NIL
    (PROGN
    #'(LAMBDA (CONDITION) (RETURN-FROM #:G2726 (VALUES NIL CONDITION)))
    ) ) )
    (#:G2728 #'(LAMBDA NIL (PROGN (= X Y))))
    )
    (LOCALLY (DECLARE (COMPILE))
    (SYSTEM::%HANDLER-BIND #:G2728 'ERROR
    #'(LAMBDA (CONDITION) (FUNCALL (FUNCALL #:G2727) CONDITION))
    ) ) )

    is not compiled to good code. This needs work in the compiler.

    A smaller example is:
    (disassemble (LAMBDA (X Y) (HANDLER-BIND () (= X Y))))

    Macro expansion:

    (LET ((#:G2784 #'(LAMBDA NIL (PROGN (= X Y)))))
    (LOCALLY (DECLARE (COMPILE)) (SYSTEM::%HANDLER-BIND #:G2784))
    )

    clisp's function inliner is not powerful enough to resolve this. I don't remember whether it is because
    it does not do data flow analysis or because the LOCALLY form introduces a new declaration scope.
    But there is a way to make this special case work better without beefing up the function inliner nor the
    data flow analysis...

     
  • Sam Steingold
    Sam Steingold
    2011-03-28

    the simple version
    (defmacro handler-bind (clauses &body body)
    `(LOCALLY (DECLARE (COMPILE))
    (SYS::%HANDLER-BIND
    #'(LAMBDA () ,@body)
    ,@(mapcan #'(lambda (clause)
    (destructuring-bind (typespec handler) clause
    `(',typespec ,handler)))
    clauses))))
    does not work.

    before the patch c-handler-bind received the non-macro-expanded version which made all it possible to inline everything.

     
  • Sam Steingold
    Sam Steingold
    2011-03-29

    thank you for your bug report.
    the bug has been fixed in the source tree (mercurial/hg).
    you can either wait for the next release (recommended)
    or check out the current mercurial tree (see http://clisp.org\)
    and build CLISP from the sources (be advised that between
    releases the source tree is very unstable and may not even build
    on your platform).

     
  • Sam Steingold
    Sam Steingold
    2011-03-29

    this fix reintroduced bug#3258485

     
  • Sam Steingold
    Sam Steingold
    2011-03-29

    • summary: too much consing when compiling IGNORE-ERRORS --> too much consing in IGNORE-ERRORS
    • status: open --> closed-fixed