Re: [Chicken-hackers] [PATCH] Fix crash in vector-resize when making vectors smaller

[Jörg F. Wittenberger]

-(define (##sys#grow-vector v n init)
-  (let ([v2 (##sys#make-vector n init)]
-	[len (##sys#size v)] )
-    (do ([i 0 (fx+ i 1)])
-	((fx>= i len) v2)
+(define (##sys#vector-resize v n init)
+  (let ((v2 (##sys#make-vector n init))
+	(len (##sys#size v)) )
+    (do ((i 0 (fx+ i 1)))
+	((or (fx>= i n) (fx>= i len)) v2)
       (##sys#setslot v2 i (##sys#slot v i)) ) ) )

Would'nt it be much smarter to lift the limit calculation out of the do-loop?

Like this:

-(define (##sys#grow-vector v n init)
-  (let ([v2 (##sys#make-vector n init)]
-	[len (##sys#size v)] )
-    (do ([i 0 (fx+ i 1)])
-	((fx>= i len) v2)
+(define (##sys#vector-resize v n init)
+  (let ((v2 (##sys#make-vector n init))
+	(len (min (##sys#size v) n)) )
+    (do ((i 0 (fx+ i 1)))
	((fx>= i len) v2)
       (##sys#setslot v2 i (##sys#slot v i)) ) ) )

???

After all vector-resizing is a frequently called procedure.  I doubt that one should neglect performance impact in it's context.

Especially not when the alternative is actually simpler.  ;-)

/Jörg

Am 17.01.2014 21:32, schrieb Peter Bex:

Hi all,

While working on something else I noticed that if you supply the
resize-vector with a smaller target vector size than the current
vector, it will cause a crash with paranoid checks enabled (DEBUGBUILD=1).

Attached is a simple patch + test.  The ##sys#grow-vector calls in
library.scm itself really only increase the vector size, so we could
leave it in, but I think the performance benefit is neglible compared
to the maintenance overhead of having two vector resizing operations.

I strongly recommend adding this to the stability branch.

Cheers,
Peter

_______________________________________________
Chicken-hackers mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/chicken-hackers