[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [sc-dev] OSCFunc oneShot is breakable because of careless FunctionList implementation



On Monday, December 9, 2013 5:59:33 PM HKT, Scott Wilson wrote:
- add a 'removeLater' method. This would defer removes to a cleanup stage called at the end of the value methods. We can't modify removeFunc to do this as it needs to return the modified list. The downside of this is it only works if functions play well with modifying their lists.

Here's a patch implementing Scott's second suggestion -- to save the functions to remove, and remove them after evaluating the FunctionList is finished. It should behave identically if you removeFunc outside of FunctionList evaluation. Inside a .value call, it may leave you with a FunctionList of one function. I think that's not so terrible.

In the most basic FunctionList scenario (when none of the functions removes a function from the list), overhead should be minimal: merely assigning a Boolean variable twice. If there is a removeFunc call during evaluation, it will create an array and garbage-collect it. I think this is a relatively rare case, and even that is not much overhead. I think this answers my concern about copying and garbage-collecting arrays for a high rate of incoming OSC messages.

Unfortunately I'm having another "git moment" -- I have fetched from upstream, merged into master, but for some reason I still have the old ScIDE.sc file with the Document redirect. QGit shows me the document-related commits, but the content is not there. (Who has time for this? Seriously. Me to git: "Please use the newest content." Git to me: "No, I won't." Me to git: "Maybe I should use mercurial from now on.")

	prCleanup {
		if(toRemove.size > 0) {
			array.removeAll(toRemove);
			toRemove = nil;
		};
		evaluating = false;
	}

I tested against my original Function:play example, and it does resolve that problem.

Another test:

f = { "func f".postln };
g = { "func g".postln };

(
var f2 = f <> { a.removeFunc(f2) },
g2 = g <> { a.slotAt(\array).debug("array") };
a = FunctionList([f2, g2]);
)

a.value;
-->
func f
array: [ a Function, a Function ]  // 'array' isn't modified yet
func g
[ func f, func g ]  // 'value' result is as it should be

a.slotAt(\array);  // 'value' has finished: 1 item remains

a.value;  // only g2 runs on the next evaluation

hjh
From 551285bf9779b6bfb918a0f42be9493875b89a8b Mon Sep 17 00:00:00 2001
From: James Harkins <jamshark70@xxxxxxxxxxxxxxxxx>
Date: Tue, 10 Dec 2013 14:13:22 +0800
Subject: [PATCH] Classlib: FunctionList: removeFunc during execution should
 wait to remove

---
 SCClassLibrary/Common/Core/AbstractFunction.sc |   46 ++++++++++++++++++------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/SCClassLibrary/Common/Core/AbstractFunction.sc b/SCClassLibrary/Common/Core/AbstractFunction.sc
index 824a081..17ea626 100644
--- a/SCClassLibrary/Common/Core/AbstractFunction.sc
+++ b/SCClassLibrary/Common/Core/AbstractFunction.sc
@@ -215,7 +215,7 @@ AbstractFunction {
 	}
 
 	// embed in ugen graph
-	asUGenInput { arg for; this.asCompileString.debug("asUGenInput"); ^this.value(for) }
+	asUGenInput { arg for; ^this.value(for) }
 	asAudioRateInput { |for|
 		var result = this.value(for);
 		^if(result.rate != \audio) { K2A.ar(result) } { result }
@@ -313,6 +313,7 @@ NAryOpFunction : AbstractFunction {
 
 FunctionList : AbstractFunction {
 	var <>array, <flopped=false;
+	var evaluating = false, toRemove;
 
 	*new { arg functions;
 		^super.newCopyArgs(functions)
@@ -322,8 +323,19 @@ FunctionList : AbstractFunction {
 		array = array.addAll(functions)
 	}
 	removeFunc { arg function;
-		array.remove(function);
-		if(array.size < 2) { ^array[0] };
+		if(evaluating) {
+			toRemove = toRemove.add(function);
+		} {
+			array.remove(function);
+			if(array.size < 2) { ^array[0] };
+		};
+	}
+	prCleanup {
+		if(toRemove.size > 0) {
+			array.removeAll(toRemove);
+			toRemove = nil;
+		};
+		evaluating = false;
 	}
 
 	replaceFunc { arg find, replace;
@@ -331,29 +343,43 @@ FunctionList : AbstractFunction {
 	}
 
 	value { arg ... args;
-		var res = array.collect(_.valueArray(args));
+		var res;
+		evaluating = true;
+		res = array.collect(_.valueArray(args));
+		this.prCleanup;
 		^if(flopped) { res.flop } { res }
 	}
 	valueArray { arg args;
-		var res = array.collect(_.valueArray(args));
+		var res;
+		evaluating = true;
+		res = array.collect(_.valueArray(args));
+		this.prCleanup;
 		^if(flopped) { res.flop } { res }
 	}
 	valueEnvir { arg ... args;
-		var res = array.collect(_.valueArrayEnvir(args));
+		var res;
+		evaluating = true;
+		res = array.collect(_.valueArrayEnvir(args));
+		this.prCleanup;
 		^if(flopped) { res.flop } { res }
 	}
 	valueArrayEnvir { arg args;
-		var res = array.collect(_.valueArrayEnvir(args));
+		var res;
+		evaluating = true;
+		res = array.collect(_.valueArrayEnvir(args));
+		this.prCleanup;
 		^if(flopped) { res.flop } { res }
 	}
 	do { arg function;
-		array.do(function)
+		evaluating = true;
+		array.do(function);
+		this.prCleanup;
 	}
 	flop {
-		if(flopped.not) {array = array.collect(_.flop) }; flopped = true;
+		if(flopped.not) {array = array.copy.collect(_.flop) }; flopped = true;
 	}
 	envirFlop {
-		if(flopped.not) {array = array.collect(_.envirFlop) }; flopped = true;
+		if(flopped.not) {array = array.copy.collect(_.envirFlop) }; flopped = true;
 	}
 	storeArgs { ^[array] }
 }
-- 
1.7.9.5