unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* [Patch] Re-implement srfi-1 partition in C to avoid stack overflow
@ 2003-06-19 15:58 Matthias Koeppe
  2003-07-09 23:11 ` Kevin Ryde
  2003-07-13 23:06 ` Kevin Ryde
  0 siblings, 2 replies; 4+ messages in thread
From: Matthias Koeppe @ 2003-06-19 15:58 UTC (permalink / raw)


The partition procedure in srfi-1 does not work well in Guile.  Even
for not-very-long input lists (like 500 elements), a stack overflow is
signaled.  The reason seems to be the recursive use of receive and
values.

Here is the srfi/ChangeLog entry:

2003-06-19  Matthias Koeppe  <mkoeppe@mail.math.uni-magdeburg.de>

	* srfi-1.c (scm_srfi1_partition), srfi-1.scm (partition): 
	Re-implement in C to avoid stack overflows for long input lists.

Index: test-suite/tests/srfi-1.test
===================================================================
RCS file: /cvs/guile/guile-core/test-suite/tests/srfi-1.test,v
retrieving revision 1.2
diff -u -c -r1.2 srfi-1.test
*** test-suite/tests/srfi-1.test	12 May 2003 23:05:50 -0000	1.2
--- test-suite/tests/srfi-1.test	19 Jun 2003 15:52:41 -0000
***************
*** 183,185 ****
--- 183,229 ----
    (pass-if "'(a b . c) 2"
      (equal? '(a b)
  	    (take '(a b . c) 2))))
+ 
+ ;;
+ ;; partition
+ ;;
+ 
+ (define (test-partition pred list kept-good dropped-good)
+   (call-with-values (lambda ()
+ 			(partition pred list))
+       (lambda (kept dropped)
+ 	(and (equal? kept kept-good)
+ 	     (equal? dropped dropped-good)))))
+ 
+ (with-test-prefix "partition"
+ 		  
+   (pass-if "with dropped tail"
+     (test-partition even? '(1 2 3 4 5 6 7)
+ 		    '(2 4 6) '(1 3 5 7)))
+ 
+   (pass-if "with kept tail"
+     (test-partition even? '(1 2 3 4 5 6)
+ 		    '(2 4 6) '(1 3 5)))
+ 
+   (pass-if "with everything dropped"
+     (test-partition even? '(1 3 5 7)
+ 		    '() '(1 3 5 7)))
+ 
+   (pass-if "with everything kept"
+     (test-partition even? '(2 4 6)
+ 		    '(2 4 6) '()))
+ 
+   (pass-if "with empty list"
+     (test-partition even? '()
+ 		    '() '()))
+ 
+   (pass-if "with reasonably long list"
+     ;; the old implementation from SRFI-1 reference implementation
+     ;; would signal a stack-overflow for a list of only 500 elements!
+     (call-with-values (lambda ()
+ 			(partition even?
+ 				   (make-list 10000 1)))
+       (lambda (even odd)
+ 	(and (= (length odd) 10000)
+ 	     (= (length even) 0))))))
+ 
Index: srfi/srfi-1.c
===================================================================
RCS file: /cvs/guile/guile-core/srfi/srfi-1.c,v
retrieving revision 1.6
diff -u -c -r1.6 srfi-1.c
*** srfi/srfi-1.c	21 Apr 2003 01:59:57 -0000	1.6
--- srfi/srfi-1.c	19 Jun 2003 15:52:41 -0000
***************
*** 319,324 ****
--- 319,364 ----
  }
  #undef FUNC_NAME
  
+ SCM_DEFINE (scm_srfi1_partition, "partition", 2, 0, 0,
+ 	    (SCM pred, SCM list),
+ 	    "Partition the elements of @var{list} with predicate @var{pred}.\n"
+ 	    "Return two values: the list of elements satifying @var{pred} and\n"
+ 	    "the list of elements @emph{not} satisfying @var{pred}.  The order\n"
+ 	    "of the output lists follows the order of @var{list}.  @var{list}\n"
+ 	    "is not mutated.  One of the output lists may share memory with @var{list}.\n")
+ #define FUNC_NAME s_scm_srfi1_partition
+ {
+   /* In this implementation, the output lists don't share memory with
+      list, because it's probably not worth the effort. */
+   scm_t_trampoline_1 call = scm_trampoline_1(pred);
+   SCM kept = scm_cons(SCM_EOL, SCM_EOL);
+   SCM kept_tail = kept;
+   SCM dropped = scm_cons(SCM_EOL, SCM_EOL);
+   SCM dropped_tail = dropped;
+   
+   SCM_ASSERT(call, pred, 2, FUNC_NAME);
+   
+   for (; !SCM_NULL_OR_NIL_P (list); list = SCM_CDR(list)) {
+     SCM elt = SCM_CAR(list);
+     SCM new_tail = scm_cons(SCM_CAR(list), SCM_EOL);
+     if (SCM_NFALSEP(call(pred, elt))) {
+       SCM_SETCDR(kept_tail, new_tail);
+       kept_tail = new_tail;
+     }
+     else {
+       SCM_SETCDR(dropped_tail, new_tail);
+       dropped_tail = new_tail;
+     }
+   }
+   /* re-use the initial conses for the values list */
+   SCM_SETCAR(kept, SCM_CDR(kept));
+   SCM_SETCDR(kept, dropped);
+   SCM_SETCAR(dropped, SCM_CDR(dropped));
+   SCM_SETCDR(dropped, SCM_EOL);
+   return scm_values(kept);
+ }
+ #undef FUNC_NAME
+ 
  void
  scm_init_srfi_1 (void)
  {
Index: srfi/srfi-1.scm
===================================================================
RCS file: /cvs/guile/guile-core/srfi/srfi-1.scm,v
retrieving revision 1.24
diff -u -c -r1.24 srfi-1.scm
*** srfi/srfi-1.scm	12 May 2003 23:02:01 -0000	1.24
--- srfi/srfi-1.scm	19 Jun 2003 15:52:41 -0000
***************
*** 662,676 ****
  
  ;;; Filtering & partitioning
  
- (define (partition pred list)
-   (if (null? list)
-     (values '() '())
-     (if (pred (car list))
-       (receive (in out) (partition pred (cdr list))
- 	       (values (cons (car list) in) out))
-       (receive (in out) (partition pred (cdr list))
- 	       (values in (cons (car list) out))))))
- 
  (define (remove pred list)
    (filter (lambda (x) (not (pred x))) list))
  
--- 662,667 ----


-- 
Matthias Koeppe -- http://www.math.uni-magdeburg.de/~mkoeppe


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Patch] Re-implement srfi-1 partition in C to avoid stack overflow
  2003-06-19 15:58 [Patch] Re-implement srfi-1 partition in C to avoid stack overflow Matthias Koeppe
@ 2003-07-09 23:11 ` Kevin Ryde
  2003-07-10 13:59   ` Matthias Koeppe
  2003-07-13 23:06 ` Kevin Ryde
  1 sibling, 1 reply; 4+ messages in thread
From: Kevin Ryde @ 2003-07-09 23:11 UTC (permalink / raw)
  Cc: guile-devel

Matthias Koeppe <mkoeppe@merkur.math.uni-magdeburg.de> writes:
>
> The partition procedure in srfi-1 does not work well in Guile.  Even
> for not-very-long input lists (like 500 elements), a stack overflow is
> signaled.  The reason seems to be the recursive use of receive and
> values.

There a few unfortunate places like that.  I've been having a bit of a
go (not yet quite ready) at new alist-delete, alist-delete! and
alist-copy to that end too.

append-map looks like another, and count (the general case, not
count1) if I'm not mistaken.

> +   /* In this implementation, the output lists don't share memory with
> +      list, because it's probably not worth the effort. */

It might be nice to share everywhere it's permitted, if you could be
bothered.  (The non-tail recursion you're fixing is clearly the worst
problem though.)


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Patch] Re-implement srfi-1 partition in C to avoid stack overflow
  2003-07-09 23:11 ` Kevin Ryde
@ 2003-07-10 13:59   ` Matthias Koeppe
  0 siblings, 0 replies; 4+ messages in thread
From: Matthias Koeppe @ 2003-07-10 13:59 UTC (permalink / raw)


Kevin Ryde <user42@zip.com.au> writes:

> Matthias Koeppe <mkoeppe@merkur.math.uni-magdeburg.de> writes:
>>
>> +   /* In this implementation, the output lists don't share memory with
>> +      list, because it's probably not worth the effort. */
>
> It might be nice to share everywhere it's permitted, if you could be
> bothered.  (The non-tail recursion you're fixing is clearly the worst
> problem though.)

I decided against implementing the sharing of structure because:

1) It has a performance penalty because parts of the list need to be
   traversed twice.

2) The savings by the sharing of structure are "unpredictable", as it
   depends on the order of the input elements.  There could only be
   savings if there is a long list tail of elements that go into one
   of the output lists, which I think is an "unlikely" case.

On the other hand, it would be worthwhile to implement partition! so
that it re-uses the cons cells of the input list.  However, I don't
have time to work on it.

-- 
Matthias Köppe -- http://www.math.uni-magdeburg.de/~mkoeppe


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Patch] Re-implement srfi-1 partition in C to avoid stack overflow
  2003-06-19 15:58 [Patch] Re-implement srfi-1 partition in C to avoid stack overflow Matthias Koeppe
  2003-07-09 23:11 ` Kevin Ryde
@ 2003-07-13 23:06 ` Kevin Ryde
  1 sibling, 0 replies; 4+ messages in thread
From: Kevin Ryde @ 2003-07-13 23:06 UTC (permalink / raw)
  Cc: guile-devel

Matthias Koeppe <mkoeppe@merkur.math.uni-magdeburg.de> writes:
>
> 	* srfi-1.c (scm_srfi1_partition), srfi-1.scm (partition): 
> 	Re-implement in C to avoid stack overflows for long input lists.

Since no-one has objected, I made an executive decision and checked
this in.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2003-07-13 23:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-06-19 15:58 [Patch] Re-implement srfi-1 partition in C to avoid stack overflow Matthias Koeppe
2003-07-09 23:11 ` Kevin Ryde
2003-07-10 13:59   ` Matthias Koeppe
2003-07-13 23:06 ` Kevin Ryde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).