Subject: [Sbcl-devel] PCL depessimization
From: Christophe Rhodes <[email-masked]>
To: SBCL <[email-masked]>
Date: Thu, 03 Feb 2005 12:20:05 +0000
List-Id: <sbcl-devel.lists.sourceforge.net>

Hi,

At present, when SBCL sees a method such as
  (defmethod bar ((x foo)) (slot-value x 'x1))
it generates a method function which internally contains something like
  (let ((x (nth 2 args)))
    (declare (type foo x))
    ... (slot-value x 'x1))
where I am sweeping quite a lot of complexity under the rug.

The attached patch inserts the type declaration for x in this and
similar circumstances more selectively: it only does so when the
parameter specializer names a built-in-class or structure-class;
specifically, it no longer does so for clos classes, and it
_definitely_ doesn't do so any more for forthcoming-clos-classes --
classes which have been noted but not yet defined (usually defined
earlier in the same file).

Why not?  Well, a type check for a clos class is really quite
expensive -- involving classoid-typep and classoid-cell-typep.  A type
check for a clos class which hasn't even been defined yet is insanely
expensive -- involving a full call to %TYPEP and SPECIFIER-TYPE.  This
is especially so for applications which have their functionality built
up from many small methods -- each method function has its own type
check.  This is particularly annoying because for the 99.9999% of
times when standard method combination or one of the built-in method
combination types is used, the declaration is trivially true.

So I propose to remove these declarations as in the attached patch,
but just before I do, I wanted to check that no-one is relying on them
to detect bad user-defined method combinations or possibly bad calls
to call-next-method; however, the effect on a simple McCLIM
application is so noticeable, even on a very fast machine, that I
suspect that even if some kind of checking is required this is not the
mechanism to ensure it.  (I'd appreciate it if someone else felt like
trying their clim or other clos-based application with this patch,
just to reassure me that I'm not hallucinating the massive speedups).

? contrib/sb-simple-streams/test-data.tmp
Index: src/pcl/boot.lisp
===================================================================
RCS file: /cvsroot/sbcl/sbcl/src/pcl/boot.lisp,v
retrieving revision 1.89
diff -u -r1.89 boot.lisp
--- src/pcl/boot.lisp	13 Jan 2005 10:12:19 -0000	1.89
+++ src/pcl/boot.lisp	3 Feb 2005 12:11:47 -0000
@@ -614,23 +614,28 @@
 	 ;; Otherwise, we can usually make Python very happy.
 	 (let ((type (info :type :kind specializer)))
 	   (ecase type
-	     ((:primitive :defined :instance :forthcoming-defclass-type)
-	      `(type ,specializer ,parameter))
-	     ((nil)
+	     ((:primitive) `(type ,specializer ,parameter))
+	     ((:instance nil)
 	      (let ((class (find-class specializer nil)))
-		(if class
-		    `(type ,(class-name class) ,parameter)
-		    (progn
-		      ;; we can get here, and still not have a failure
-		      ;; case, by doing MOP programming like (PROGN
-		      ;; (ENSURE-CLASS 'FOO) (DEFMETHOD BAR ((X FOO))
-		      ;; ...)).  Best to let the user know we haven't
-		      ;; been able to extract enough information:
-		      (style-warn
-		       "~@<can't find type for presumed class ~S in ~S.~@:>"
-		       specializer
-		       'parameter-specializer-declaration-in-defmethod)
-		      '(ignorable))))))))))
+		(cond
+		  (class
+		   (if (typep class '(or built-in-class structure-class))
+		       `(type ,specializer ,parameter)
+		       ;; don't declare CLOS classes as parameters;
+		       ;; it's too expensive.
+		       '(ignorable)))
+		  (t
+		   ;; we can get here, and still not have a failure
+		   ;; case, by doing MOP programming like (PROGN
+		   ;; (ENSURE-CLASS 'FOO) (DEFMETHOD BAR ((X FOO))
+		   ;; ...)).  Best to let the user know we haven't
+		   ;; been able to extract enough information:
+		   (style-warn
+		    "~@<can't find type for presumed class ~S in ~S.~@:>"
+		    specializer
+		    'parameter-specializer-declaration-in-defmethod)
+		   '(ignorable)))))
+	     ((:forthcoming-defclass-type) '(ignorable)))))))
 
 (defun make-method-lambda-internal (method-lambda &optional env)
   (unless (and (consp method-lambda) (eq (car method-lambda) 'lambda))

Cheers,

Christophe