New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework server boot logic #3286

Open
jamshark70 opened this Issue Nov 11, 2017 · 33 comments

Comments

Projects
None yet
7 participants
@jamshark70
Contributor

jamshark70 commented Nov 11, 2017

The components of the server boot process are spread out over too many methods, in too many distinct threads, causing bugs.

Based on discussion under #3280, I propose we rip it out and rebuild it:

  • There should only ever be one thread watching for the server to come up. Currently, each invocation of doWhenBooted creates a new thread. This causes sync problems.

  • After the server comes up, this single thread should call all of the init components in sequence, synchronously. (If any component itself forks another thread, that's its own responsibility.)

  • Those components are (listed here in proposed order of execution):

    • Shared memory initialization
    • Request notification messages --> get clientID from scsynth
    • ServerBoot, used by:
      • NodeWatcher
      • SynthDescLib
      • ProxySpace
      • ServerMeterView
      • Volume
    • ServerTree, used in... lots of places
    • User functions given to waitForBoot or doWhenBooted.
  • We can't consolidate ServerBoot and ServerTree, because the latter also happens after cmd-.

  • We can't consolidate CmdPeriod and ServerTree, because one is for stopping and one is for resetting.

Still to be decided:

  • The final order of init components.
  • Do the items that add themselves to ServerBoot and ServerTree need to be added in a specific order? (Currently we are assuming they don't... and, they add themselves in *initClass, whose order we already know to be unpredictable.)
  • Should waitForBoot / doWhenBooted functions to be added to ServerTree, using a proposed doOnce method?
  • If server boot fails, what should happen to waitForBoot / doWhenBooted functions?
    • If the decision is to cancel these functions, then we need to improve the logic to detect a server process that quits during the startup sequence -- the language should recognize quickly that the server went down, instead of blindly waiting for 20 seconds.

Did I miss anything?

@muellmusik

This comment has been minimized.

Show comment
Hide comment
@muellmusik

muellmusik Nov 11, 2017

Contributor

Thanks for doing this, James.

Do the items that add themselves to ServerBoot and ServerTree need to be added in a specific order? (Currently we are assuming they don't... and, they add themselves in *initClass, whose order we already know to be unpredictable.)

IRC, items are executed in the order added. If the order matters, you could try initClassTree, or maybe better, add them in some common place (e.g. Server itself) so that order is clear. I think it's a problem of 'grainedness'...

If server boot fails, what should happen to waitForBoot / doWhenBooted functions?
If the decision is to cancel these functions, then we need to improve the logic to detect a server process that quits during the startup sequence -- the language should recognize quickly that the server went down, instead of blindly waiting for 20 seconds.

A good use for dependancy?

Contributor

muellmusik commented Nov 11, 2017

Thanks for doing this, James.

Do the items that add themselves to ServerBoot and ServerTree need to be added in a specific order? (Currently we are assuming they don't... and, they add themselves in *initClass, whose order we already know to be unpredictable.)

IRC, items are executed in the order added. If the order matters, you could try initClassTree, or maybe better, add them in some common place (e.g. Server itself) so that order is clear. I think it's a problem of 'grainedness'...

If server boot fails, what should happen to waitForBoot / doWhenBooted functions?
If the decision is to cancel these functions, then we need to improve the logic to detect a server process that quits during the startup sequence -- the language should recognize quickly that the server went down, instead of blindly waiting for 20 seconds.

A good use for dependancy?

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 11, 2017

Contributor

maybe better, add them in some common place (e.g. Server itself)...

Yes, better I think, because part of the problem is that pieces of the process are spread out so that their relationships are harder to see and to control. Distributing these ServerBoot things among multiple initClass methods is that problem in a nutshell.

I'm not clear how dependants will watch the status of an external process. Thinking about it...

Contributor

jamshark70 commented Nov 11, 2017

maybe better, add them in some common place (e.g. Server itself)...

Yes, better I think, because part of the problem is that pieces of the process are spread out so that their relationships are harder to see and to control. Distributing these ServerBoot things among multiple initClass methods is that problem in a nutshell.

I'm not clear how dependants will watch the status of an external process. Thinking about it...

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 14, 2017

Member

Thanks for the spec James, very much appreciated.

There was one thing that came up in #3280 that I was curious about, but reading it again I'm not sure I understand:

But, a question: My proposal for 3.9.1 is to put all onComplete actions into a queue, to guarantee order. Currently, these already run in the context of routines. Would you like your bootSync'ed routine to be synchronous or asynchronous WRT other boot actions?

I'm going to propose making them synchronous. If you do a waitForBoot (A), a bootSync (B) and another waitForBoot (C), I think A should 100% finish before B, which finishes before C.

Can you give some example code for this situation? Would it be something like

r {
  s.waitForBoot({ post("hello") });
  s.bootSync();
  s.waitForBoot({ post("goodbye") });
}

?

Also, I understand the desire to make things less error-prone, but I think forcing all added actions to execute in order is heavy-handed. What if you (want to) put a waiting loop inside one of your onComplete routines? I think a DAG (where edges represent completion dependencies) is a more accurate model here than a queue. Is fine-grained control with condition variables the only option right now to achieve that kind of thing? Perhaps the interface could be extended to make it easier to indicate dependencies between routines. I think it would not be too hard to implement either. In each routine that wraps an onComplete action:

  1. At the end, signal a condition variable associated with that action.
  2. At the start, wait on all the condition variables named as dependencies.
Member

brianlheim commented Nov 14, 2017

Thanks for the spec James, very much appreciated.

There was one thing that came up in #3280 that I was curious about, but reading it again I'm not sure I understand:

But, a question: My proposal for 3.9.1 is to put all onComplete actions into a queue, to guarantee order. Currently, these already run in the context of routines. Would you like your bootSync'ed routine to be synchronous or asynchronous WRT other boot actions?

I'm going to propose making them synchronous. If you do a waitForBoot (A), a bootSync (B) and another waitForBoot (C), I think A should 100% finish before B, which finishes before C.

Can you give some example code for this situation? Would it be something like

r {
  s.waitForBoot({ post("hello") });
  s.bootSync();
  s.waitForBoot({ post("goodbye") });
}

?

Also, I understand the desire to make things less error-prone, but I think forcing all added actions to execute in order is heavy-handed. What if you (want to) put a waiting loop inside one of your onComplete routines? I think a DAG (where edges represent completion dependencies) is a more accurate model here than a queue. Is fine-grained control with condition variables the only option right now to achieve that kind of thing? Perhaps the interface could be extended to make it easier to indicate dependencies between routines. I think it would not be too hard to implement either. In each routine that wraps an onComplete action:

  1. At the end, signal a condition variable associated with that action.
  2. At the start, wait on all the condition variables named as dependencies.
@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 14, 2017

Contributor

Oh, I see, I might have overstated that case a bit.

I was referring to the fact that waitForBoot actions already run in the context of a routine, so you can already include wait in them, directly.

So, if you write something like this:

s.waitForBoot {
	... do something A...
	1.wait;
	... do something else B...
};

s.waitForBoot {
	... do a third something C...
	1.wait;
	... do something number 4 D...
};

... you could be absolutely certain that the order of execution would be A, B, C, D. As it is now, the two threads will swap control, so the order is A, C, B, D.

Now, if the user forks a totally separate thread inside a waitForBoot function, I think waitForBoot should not be responsible for tracking that thread and blocking the server init Routine until all subthreads finish. Of course not... if you fork something, we should assume you know what you're doing.

I just realized that mixing bootSync with waitForBoot makes it impossible to control order of execution, because bootSync must, by definition, be used in a user-created Routine and, by definition, that will be outside of the control of the server-init thread. So, if a user needs fine-grained control over the order of her own server init actions, she should use either waitForBoot or bootSync, but not both. That's not a difficult requirement to meet, since using both would violate Occam's Razor.

Contributor

jamshark70 commented Nov 14, 2017

Oh, I see, I might have overstated that case a bit.

I was referring to the fact that waitForBoot actions already run in the context of a routine, so you can already include wait in them, directly.

So, if you write something like this:

s.waitForBoot {
	... do something A...
	1.wait;
	... do something else B...
};

s.waitForBoot {
	... do a third something C...
	1.wait;
	... do something number 4 D...
};

... you could be absolutely certain that the order of execution would be A, B, C, D. As it is now, the two threads will swap control, so the order is A, C, B, D.

Now, if the user forks a totally separate thread inside a waitForBoot function, I think waitForBoot should not be responsible for tracking that thread and blocking the server init Routine until all subthreads finish. Of course not... if you fork something, we should assume you know what you're doing.

I just realized that mixing bootSync with waitForBoot makes it impossible to control order of execution, because bootSync must, by definition, be used in a user-created Routine and, by definition, that will be outside of the control of the server-init thread. So, if a user needs fine-grained control over the order of her own server init actions, she should use either waitForBoot or bootSync, but not both. That's not a difficult requirement to meet, since using both would violate Occam's Razor.

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 14, 2017

Member

... you could be absolutely certain that the order of execution would be A, B, C, D. As it is now, the two threads will swap control, so the order is A, C, B, D.

OK. My question is, why would you want that, without exception, all the time? That makes this code impossible:

s.waitForBoot {
	loop {
		1.wait;
		doA();
	}
};

s.waitForBoot {
	loop {
		2.0.rand.wait;
		doB();
	}
};

But this is still possible if things are kept as-is:

(
var bCond = Condition();

s.waitForBoot {
	"do A".postln;
	1.wait;
	"do B".postln;
	bCond.test_(true).signal;
};

s.waitForBoot {
	bCond.wait();
	"do C".postln;
	1.wait;
	"do D".postln;
};
)
Member

brianlheim commented Nov 14, 2017

... you could be absolutely certain that the order of execution would be A, B, C, D. As it is now, the two threads will swap control, so the order is A, C, B, D.

OK. My question is, why would you want that, without exception, all the time? That makes this code impossible:

s.waitForBoot {
	loop {
		1.wait;
		doA();
	}
};

s.waitForBoot {
	loop {
		2.0.rand.wait;
		doB();
	}
};

But this is still possible if things are kept as-is:

(
var bCond = Condition();

s.waitForBoot {
	"do A".postln;
	1.wait;
	"do B".postln;
	bCond.test_(true).signal;
};

s.waitForBoot {
	bCond.wait();
	"do C".postln;
	1.wait;
	"do D".postln;
};
)
@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 15, 2017

Contributor

OK. My question is, why would you want that, without exception, all the time? That makes this code impossible:

s.waitForBoot {
	loop {
		1.wait;
		doA();
	}
};

s.waitForBoot {
	loop {
		2.0.rand.wait;
		doB();
	}
};

Interesting. I find the example to be unclear, though. It's forking routines, but not explicitly forking them. (I.e., where you're asking "why would you want that?", I find myself asking, why on earth would you write it as you suggested?) I tend to think, write what you mean: after booting the server, start some process running:

s.waitForBoot {
	fork {
		loop {
			...
		}
	};
};

And then you could even dispense with the second waitForBoot, and put both forks in the same block.

Neither approach prohibits any functionality. I do think my proposal has the advantage that the simpler use case (do things in a given sequence) is written with simpler code, and the more complex use case (do several things simultaneously in multiple forked threads) uses more complex code. I see what you're doing with the condition in your example, but it just looks weird to me that the simple idea (one thing after another) would require all that extra work.

I completely admit, that's a matter of coding style preference.

Then again, one should really just use one danged waitForBoot and avoid the whole mess.

Contributor

jamshark70 commented Nov 15, 2017

OK. My question is, why would you want that, without exception, all the time? That makes this code impossible:

s.waitForBoot {
	loop {
		1.wait;
		doA();
	}
};

s.waitForBoot {
	loop {
		2.0.rand.wait;
		doB();
	}
};

Interesting. I find the example to be unclear, though. It's forking routines, but not explicitly forking them. (I.e., where you're asking "why would you want that?", I find myself asking, why on earth would you write it as you suggested?) I tend to think, write what you mean: after booting the server, start some process running:

s.waitForBoot {
	fork {
		loop {
			...
		}
	};
};

And then you could even dispense with the second waitForBoot, and put both forks in the same block.

Neither approach prohibits any functionality. I do think my proposal has the advantage that the simpler use case (do things in a given sequence) is written with simpler code, and the more complex use case (do several things simultaneously in multiple forked threads) uses more complex code. I see what you're doing with the condition in your example, but it just looks weird to me that the simple idea (one thing after another) would require all that extra work.

I completely admit, that's a matter of coding style preference.

Then again, one should really just use one danged waitForBoot and avoid the whole mess.

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 15, 2017

Member

Interesting. I find the example to be unclear, though. It's forking routines, but not explicitly forking them. (I.e., where you're asking "why would you want that?", I find myself asking, why on earth would you write it as you suggested?)

What is unclear or lacking in meaning about it? Each action is looping infinitely. It's exactly like the examples you gave, just with a loop stuck in it. This makes perfect sense to me, so I don't know how to respond to this.

I do think my proposal has the advantage that the simpler use case (do things in a given sequence) is written with simpler code, and the more complex use case (do several things simultaneously in multiple forked threads) uses more complex code.

You're going to have to convince me that a situation where you have multiple interacting non-thread-safe actions is the simpler use case.

Neither approach prohibits any functionality.

True, I was not thinking clearly before. I guess having a thread whose only purpose is to fork out into another thread seems counterintuitive to me. As an aside, won't this potentially break a lot of code?

I see what you're doing with the condition in your example, but it just looks weird to me that the simple idea (one thing after another) would require all that extra work.

Unfortunately, that is what it takes to do multithreading properly. Or, as you already pointed out above, you could just put it all in the same routine, and then literally have one thing after the other. :)

Like I said, you could make this simpler with an interface that essentially inserts those condition variables and the waiting/signaling functionality behind the scenes, only when needed. The only limitation in that case would be that you have to build the DAG in order of dependencies, but that is no stricter than the requirement for the queue.

Member

brianlheim commented Nov 15, 2017

Interesting. I find the example to be unclear, though. It's forking routines, but not explicitly forking them. (I.e., where you're asking "why would you want that?", I find myself asking, why on earth would you write it as you suggested?)

What is unclear or lacking in meaning about it? Each action is looping infinitely. It's exactly like the examples you gave, just with a loop stuck in it. This makes perfect sense to me, so I don't know how to respond to this.

I do think my proposal has the advantage that the simpler use case (do things in a given sequence) is written with simpler code, and the more complex use case (do several things simultaneously in multiple forked threads) uses more complex code.

You're going to have to convince me that a situation where you have multiple interacting non-thread-safe actions is the simpler use case.

Neither approach prohibits any functionality.

True, I was not thinking clearly before. I guess having a thread whose only purpose is to fork out into another thread seems counterintuitive to me. As an aside, won't this potentially break a lot of code?

I see what you're doing with the condition in your example, but it just looks weird to me that the simple idea (one thing after another) would require all that extra work.

Unfortunately, that is what it takes to do multithreading properly. Or, as you already pointed out above, you could just put it all in the same routine, and then literally have one thing after the other. :)

Like I said, you could make this simpler with an interface that essentially inserts those condition variables and the waiting/signaling functionality behind the scenes, only when needed. The only limitation in that case would be that you have to build the DAG in order of dependencies, but that is no stricter than the requirement for the queue.

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 15, 2017

Contributor

Past experience, the quote-reply-quote-reply format risks becoming a tit-for-tat argument rather than a collaboration ("You're going to have to convince me..." etc.). So, I won't do that.

Except one thing: "As an aside, won't this potentially break a lot of code?" Right, it would break an unknown amount of code. From reading the mailing lists, I can say I haven't seen a lot of users apply waitForBoot as a synonym for boot+fork, for looping purposes. The vast majority of waitForBoot cases don't even use wait inside, so I don't think it would be a lot. But, yes, we do have to take that risk into account, and I haven't forgotten about it.

waitForBoot queuing is, in the end, a side issue. The main point of reworking server boot logic is that several of the stages seem to be called from different status-watching threads, which happen to fire in a more-or-less sensible order, but which order is not strictly guaranteed by anything in the code. My main priority in opening this issue is to make sure that shared memory init, notification, client ID receipt, allocator init, ServerBoot, ServerTree and user actions (taking user actions as a group) fire in a 100% deterministic, clear order that cannot get reordered by threading glitches.

This is reacting to specific problems such as creating allocators, firing a user action, and then re-creating allocators after the client ID comes back (so that the next node gets a duplicate ID).

If a user wants to call waitForBoot a dozen times, I suppose it's enough to guarantee that the threads will be forked in the order of the waitForBoot calls, and all other synchronization is the user's own responsibility/risk. I won't object to that too strongly.

In the current implementation, we can't be 100% sure of that because the user action runs at the tail end of a thread that starts off by waiting for the server to boot. That is, currently we have "multiple interacting non-thread-safe actions" as the default situation. I'm not trying to convince you that this is simple because my preference would be exactly the opposite: NO implicit threading for the user's server-boot actions. If I had my way: If the user wants threading, the user can write it, explicitly, and take responsibility for it -- instead of the current implementation, where the user can be caught unawares by thread-sync problems.

But that may simply be a documentation problem -- caveat emptor, if you call waitForBoot a bunch of times, it's up to you a/ to be aware things might not happen in the order you expect and b/ to sync them if you need them in a specific order. I'd actually be OK with that. (But I still think it's misleading to thread them behind the user's back.)

Done for today. I have classes to prepare.

Contributor

jamshark70 commented Nov 15, 2017

Past experience, the quote-reply-quote-reply format risks becoming a tit-for-tat argument rather than a collaboration ("You're going to have to convince me..." etc.). So, I won't do that.

Except one thing: "As an aside, won't this potentially break a lot of code?" Right, it would break an unknown amount of code. From reading the mailing lists, I can say I haven't seen a lot of users apply waitForBoot as a synonym for boot+fork, for looping purposes. The vast majority of waitForBoot cases don't even use wait inside, so I don't think it would be a lot. But, yes, we do have to take that risk into account, and I haven't forgotten about it.

waitForBoot queuing is, in the end, a side issue. The main point of reworking server boot logic is that several of the stages seem to be called from different status-watching threads, which happen to fire in a more-or-less sensible order, but which order is not strictly guaranteed by anything in the code. My main priority in opening this issue is to make sure that shared memory init, notification, client ID receipt, allocator init, ServerBoot, ServerTree and user actions (taking user actions as a group) fire in a 100% deterministic, clear order that cannot get reordered by threading glitches.

This is reacting to specific problems such as creating allocators, firing a user action, and then re-creating allocators after the client ID comes back (so that the next node gets a duplicate ID).

If a user wants to call waitForBoot a dozen times, I suppose it's enough to guarantee that the threads will be forked in the order of the waitForBoot calls, and all other synchronization is the user's own responsibility/risk. I won't object to that too strongly.

In the current implementation, we can't be 100% sure of that because the user action runs at the tail end of a thread that starts off by waiting for the server to boot. That is, currently we have "multiple interacting non-thread-safe actions" as the default situation. I'm not trying to convince you that this is simple because my preference would be exactly the opposite: NO implicit threading for the user's server-boot actions. If I had my way: If the user wants threading, the user can write it, explicitly, and take responsibility for it -- instead of the current implementation, where the user can be caught unawares by thread-sync problems.

But that may simply be a documentation problem -- caveat emptor, if you call waitForBoot a bunch of times, it's up to you a/ to be aware things might not happen in the order you expect and b/ to sync them if you need them in a specific order. I'd actually be OK with that. (But I still think it's misleading to thread them behind the user's back.)

Done for today. I have classes to prepare.

@miguel-negrao

This comment has been minimized.

Show comment
Hide comment
@miguel-negrao

miguel-negrao Nov 15, 2017

Member

You're going to have to convince me that a situation where you have multiple interacting non-thread-safe actions is the simpler use case.

Just a comment/question: There are no non-thread-safe actions in sclang, are there ? From a previous question in sc-dev, I got the impression that while a thread is running in sclang all other threads are blocked on a mutex. Perhaps it was meant in a different sense ?

Member

miguel-negrao commented Nov 15, 2017

You're going to have to convince me that a situation where you have multiple interacting non-thread-safe actions is the simpler use case.

Just a comment/question: There are no non-thread-safe actions in sclang, are there ? From a previous question in sc-dev, I got the impression that while a thread is running in sclang all other threads are blocked on a mutex. Perhaps it was meant in a different sense ?

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 15, 2017

Contributor

There are no non-thread-safe actions in sclang, are there ? From a previous question in sc-dev, I got the impression that while a thread is running in sclang all other threads are blocked on a mutex. Perhaps it was meant in a different sense ?

That's true in the sense that the OS scheduler will not preempt one sclang Routine for another.

If we abuse the term slightly for the context of server boot, it is possible for a careless user to run multiple waitForBoot threads and not be sure of the order of evaluation.

Contributor

jamshark70 commented Nov 15, 2017

There are no non-thread-safe actions in sclang, are there ? From a previous question in sc-dev, I got the impression that while a thread is running in sclang all other threads are blocked on a mutex. Perhaps it was meant in a different sense ?

That's true in the sense that the OS scheduler will not preempt one sclang Routine for another.

If we abuse the term slightly for the context of server boot, it is possible for a careless user to run multiple waitForBoot threads and not be sure of the order of evaluation.

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 16, 2017

Member

waitForBoot queuing is, in the end, a side issue. The main point of reworking server boot logic is that several of the stages seem to be called from different status-watching threads, which happen to fire in a more-or-less sensible order, but which order is not strictly guaranteed by anything in the code. My main priority in opening this issue is to make sure that shared memory init, notification, client ID receipt, allocator init, ServerBoot, ServerTree and user actions (taking user actions as a group) fire in a 100% deterministic, clear order that cannot get reordered by threading glitches.

OK. This was just one subtopic that I wanted to talk about because I found it interesting. For the record, I think the rest of what you've proposed is solid, and I'm very glad you did.

In the current implementation, we can't be 100% sure of that because the user action runs at the tail end of a thread that starts off by waiting for the server to boot. That is, currently we have "multiple interacting non-thread-safe actions" as the default situation.

I think my terminology was not clear enough. I would say that checking on the server is thread-safe in this context, because it only involves reading state (not writing) and is surrounded with a retry loop. Conversely, any writes to the same state – and/or writes followed by reads – that are not protected by locks or conditions are not thread-safe, because they may result in different results based on different orders of execution. So, the way things are currently implemented is thread-safe under that interpretation, but of course the code the client puts into any of these functions could either maintain or break that safety.

What I meant by my original question/comment was that it's hard for me to grasp how often it's the case that users are writing code that is unsafe; whether that occurs under natural usage or not. It seems like if you're using the server's node and buffer allocators that it shouldn't happen often, but that's a total guess.

my preference would be exactly the opposite: NO implicit threading for the user's server-boot actions. If I had my way: If the user wants threading, the user can write it, explicitly, and take responsibility for it -- instead of the current implementation, where the user can be caught unawares by thread-sync problems.

If the action is meant to be performed asynchronously, doesn't it by definition have to be performed in a separate thread? Or do you mean it's implicit that each action will get its own thread?

But that may simply be a documentation problem -- caveat emptor, if you call waitForBoot a bunch of times, it's up to you a/ to be aware things might not happen in the order you expect and b/ to sync them if you need them in a specific order. I'd actually be OK with that. (But I still think it's misleading to thread them behind the user's back.)

I agree, without documentation this is nasty. It was easy for me to come into this discussion knowing the issue/behavior already and write correct code, but coming at it the other way would be difficult.

Member

brianlheim commented Nov 16, 2017

waitForBoot queuing is, in the end, a side issue. The main point of reworking server boot logic is that several of the stages seem to be called from different status-watching threads, which happen to fire in a more-or-less sensible order, but which order is not strictly guaranteed by anything in the code. My main priority in opening this issue is to make sure that shared memory init, notification, client ID receipt, allocator init, ServerBoot, ServerTree and user actions (taking user actions as a group) fire in a 100% deterministic, clear order that cannot get reordered by threading glitches.

OK. This was just one subtopic that I wanted to talk about because I found it interesting. For the record, I think the rest of what you've proposed is solid, and I'm very glad you did.

In the current implementation, we can't be 100% sure of that because the user action runs at the tail end of a thread that starts off by waiting for the server to boot. That is, currently we have "multiple interacting non-thread-safe actions" as the default situation.

I think my terminology was not clear enough. I would say that checking on the server is thread-safe in this context, because it only involves reading state (not writing) and is surrounded with a retry loop. Conversely, any writes to the same state – and/or writes followed by reads – that are not protected by locks or conditions are not thread-safe, because they may result in different results based on different orders of execution. So, the way things are currently implemented is thread-safe under that interpretation, but of course the code the client puts into any of these functions could either maintain or break that safety.

What I meant by my original question/comment was that it's hard for me to grasp how often it's the case that users are writing code that is unsafe; whether that occurs under natural usage or not. It seems like if you're using the server's node and buffer allocators that it shouldn't happen often, but that's a total guess.

my preference would be exactly the opposite: NO implicit threading for the user's server-boot actions. If I had my way: If the user wants threading, the user can write it, explicitly, and take responsibility for it -- instead of the current implementation, where the user can be caught unawares by thread-sync problems.

If the action is meant to be performed asynchronously, doesn't it by definition have to be performed in a separate thread? Or do you mean it's implicit that each action will get its own thread?

But that may simply be a documentation problem -- caveat emptor, if you call waitForBoot a bunch of times, it's up to you a/ to be aware things might not happen in the order you expect and b/ to sync them if you need them in a specific order. I'd actually be OK with that. (But I still think it's misleading to thread them behind the user's back.)

I agree, without documentation this is nasty. It was easy for me to come into this discussion knowing the issue/behavior already and write correct code, but coming at it the other way would be difficult.

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 16, 2017

Contributor

So, the way things are currently implemented is thread-safe under that interpretation, but of course the code the client puts into any of these functions could either maintain or break that safety.

Yes. Part of my thinking is to set up a clear, unambiguous dividing line between the sync / safety that the SC class library is responsible for, and the sync / safety that the user is responsible for.

If the action is meant to be performed asynchronously, doesn't it by definition have to be performed in a separate thread? Or do you mean it's implicit that each action will get its own thread?

Currently, yes, it is -- in fact! -- implicit. Look at doWhenBooted. Every time a user invokes doWhenBooted (including via waitForBoot), a new thread gets forked. The first thing that happens in that thread is a while loop, waiting for the server process to be ready. But this is not explicitly synced against any other server-init phases.

That's what I meant when I said that the current situation is implicit threading. The user gets parallelism, without asking for it, and without access to any Conditions or semaphores to handle sync.

Here's a concrete scenario. Generally, we're assuming that multiple waitForBoot invocations will happen at the same logical time. But, what if they don't? What if WFB1 happens, and then WFB2 occurs 0.1 seconds later? The while loop's periodicity is 0.2 seconds. So, there is a 50% chance that the server would become ready in the 0.1 seconds between WFB1 and WFB2 -- in which case, WFB2 would fire first, even though the user registered the WFB1 action first.

That's loony tunes, quite sloppy to fork all of these separate threads waiting for the same condition.

My suggestion is that boot would launch one single Routine, responsible for all server init. In pseudocode, the routine would look something like:

serverInitThread = Routine {
	while { serverStillBooting } {
		wait some duration
	};
	Send notification request.
	Block until client ID comes back.
	ServerBoot.
	ServerTree.
	userActionQueue.do { |func|
		func.value;
	}
}.play(AppClock);

Under this proposal, you're correct that any loops initiated by the user will block later actions. That's a breaking change. I'm not sure how extensive that impact would be (though this hasn't been a commonly reported scenario on the mailing list).

But, the user could be absolutely certain that function A would finish before function B.

If you want the user actions to be parallel, the user action queue loop would have to do fork(func, AppClock) instead. That's reasonable, actually -- but I'm still suspicious of the implicit parallelism. If the user wants parallel execution, I think the user should write it directly. Then, it's absolutely clear that it's her parallelism, and not the class library's parallelism. We are then not responsible for any sync issues. We are only responsible for launching the user actions after ServerTree has released control.

Contributor

jamshark70 commented Nov 16, 2017

So, the way things are currently implemented is thread-safe under that interpretation, but of course the code the client puts into any of these functions could either maintain or break that safety.

Yes. Part of my thinking is to set up a clear, unambiguous dividing line between the sync / safety that the SC class library is responsible for, and the sync / safety that the user is responsible for.

If the action is meant to be performed asynchronously, doesn't it by definition have to be performed in a separate thread? Or do you mean it's implicit that each action will get its own thread?

Currently, yes, it is -- in fact! -- implicit. Look at doWhenBooted. Every time a user invokes doWhenBooted (including via waitForBoot), a new thread gets forked. The first thing that happens in that thread is a while loop, waiting for the server process to be ready. But this is not explicitly synced against any other server-init phases.

That's what I meant when I said that the current situation is implicit threading. The user gets parallelism, without asking for it, and without access to any Conditions or semaphores to handle sync.

Here's a concrete scenario. Generally, we're assuming that multiple waitForBoot invocations will happen at the same logical time. But, what if they don't? What if WFB1 happens, and then WFB2 occurs 0.1 seconds later? The while loop's periodicity is 0.2 seconds. So, there is a 50% chance that the server would become ready in the 0.1 seconds between WFB1 and WFB2 -- in which case, WFB2 would fire first, even though the user registered the WFB1 action first.

That's loony tunes, quite sloppy to fork all of these separate threads waiting for the same condition.

My suggestion is that boot would launch one single Routine, responsible for all server init. In pseudocode, the routine would look something like:

serverInitThread = Routine {
	while { serverStillBooting } {
		wait some duration
	};
	Send notification request.
	Block until client ID comes back.
	ServerBoot.
	ServerTree.
	userActionQueue.do { |func|
		func.value;
	}
}.play(AppClock);

Under this proposal, you're correct that any loops initiated by the user will block later actions. That's a breaking change. I'm not sure how extensive that impact would be (though this hasn't been a commonly reported scenario on the mailing list).

But, the user could be absolutely certain that function A would finish before function B.

If you want the user actions to be parallel, the user action queue loop would have to do fork(func, AppClock) instead. That's reasonable, actually -- but I'm still suspicious of the implicit parallelism. If the user wants parallel execution, I think the user should write it directly. Then, it's absolutely clear that it's her parallelism, and not the class library's parallelism. We are then not responsible for any sync issues. We are only responsible for launching the user actions after ServerTree has released control.

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 16, 2017

Contributor

"... and without access to any Conditions or semaphores to handle sync" ... by which I mean, if we end up handling sync issues by creating a Condition or Semaphore behind the scenes, the user doesn't have access to that. That is, if it's a DAG and ServerBoot or ServerTree actions spawn completion dependencies, the user has access to sync the parts of the graph that she initiated, but no access to the parts that the class library initiated.

So I think it's better to make a clear division. Where that division lies, is open to some debate, certainly.

Contributor

jamshark70 commented Nov 16, 2017

"... and without access to any Conditions or semaphores to handle sync" ... by which I mean, if we end up handling sync issues by creating a Condition or Semaphore behind the scenes, the user doesn't have access to that. That is, if it's a DAG and ServerBoot or ServerTree actions spawn completion dependencies, the user has access to sync the parts of the graph that she initiated, but no access to the parts that the class library initiated.

So I think it's better to make a clear division. Where that division lies, is open to some debate, certainly.

@adcxyz

This comment has been minimized.

Show comment
Hide comment
@adcxyz

adcxyz Nov 16, 2017

Contributor

Dear all, haven't been following the argument, instead, I worked on sketching out a proposal
for very simple, and predictable server boot logic, with a working implementation, and tests :-)
And following @jamshark70's advice, I even wrote the unit tests first :
https://github.com/adcxyz/supercollider/tree/server_singleBootTask

Opinions welcome!

fork { TestServer_boot().test_bootSequence };
TestServer_boot.run; 
TestServer_clientID.run;
TestServer_clientID_booted.run;

My proposed order follows james' sketch, and all actions run in a single Task.
This makes the boot process deterministic and easy to reason about.

  • boot the server process -> server.hasBooted is true
  • notify, get clientID, make allocators -> serverRunning is true
  • do ServerBoot.run: everything that can or must happen before server tree:
    SynthDescLib, NodeWatcher, etc.
    sync
  • do server.initTree:
    create defaultGroups
    all user setup actions that needs server tree go into:
    s.tree
    ServerTree.run
    sync when done
  • do all tempBootItems
    user actions added by doWhenBooted, in the order they were added in.

All of them allow waiting and conditions within the functions.

// Here is a variant of test_bootSequence that works as a test on 3.8.0: 
fork {
	var a, cond, b1, b2, t1, t2, oldTFunc;
	var numBootFuncs = ServerBoot.objects !? { ServerBoot.objects[s].size } ? 0;
	var numTreeFuncs = ServerTree.objects !? { ServerTree.objects[s].size } ? 0;
	var expectedList = List[ '1_Bt', '2_Bt', '3_tr', '4_Tr', '5_Tr', '6_wt', '7_do' ];
		
	// list of function names to check
	a = List[];
	// add functions to run at all boot stages
	ServerBoot.add (b1 = { |sv1| (sv1.name + ": ").post; a.add('1_Bt'.postln); }, s);
	ServerBoot.add (b2 = { |sv2| (sv2.name + ": ").post; a.add('2_Bt'.postln); }, s);
	s.tree =        { (s.name + ": ").post; 0.1.wait; a.add('3_tr'.postln); };
	ServerTree.add (t1 = { |sv| (sv.name + ": ").post; 0.1.wait; a.add('4_Tr'.postln); }, s);
	ServerTree.add (t2 = { |sv| (sv.name + ": ").post; 0.1.wait; a.add('5_Tr'.postln); }, s);
		
	s.quit;
	a = List[];
	s.waitForBoot  { ().play; s.post; a.add('6_wt'.postln); };
	s.doWhenBooted { 0.1.wait; s.post; a.add('7_do'.postln); };
	s.doWhenBooted { 2.wait; cond.unhang; };
	s.boot;
	
	cond = Condition.new;
	cond.hang;
		
	// wait for slow late tasks, just in case
	5.wait;
	
	a.postln; 
	if (a == expectedList) { 
		"*** YES, BOOT ORDER IS CORRECT. *** ".postln;
	} { 
		"*** NO, BOOT ORDER IS not correct. *** ".postln;
	};
	
	// cleanup
	ServerBoot.remove(b1, s);
	ServerBoot.remove(b2, s);
	ServerTree.remove(t1, s);
	ServerTree.remove(t2, s);
	s.quit;
}
Contributor

adcxyz commented Nov 16, 2017

Dear all, haven't been following the argument, instead, I worked on sketching out a proposal
for very simple, and predictable server boot logic, with a working implementation, and tests :-)
And following @jamshark70's advice, I even wrote the unit tests first :
https://github.com/adcxyz/supercollider/tree/server_singleBootTask

Opinions welcome!

fork { TestServer_boot().test_bootSequence };
TestServer_boot.run; 
TestServer_clientID.run;
TestServer_clientID_booted.run;

My proposed order follows james' sketch, and all actions run in a single Task.
This makes the boot process deterministic and easy to reason about.

  • boot the server process -> server.hasBooted is true
  • notify, get clientID, make allocators -> serverRunning is true
  • do ServerBoot.run: everything that can or must happen before server tree:
    SynthDescLib, NodeWatcher, etc.
    sync
  • do server.initTree:
    create defaultGroups
    all user setup actions that needs server tree go into:
    s.tree
    ServerTree.run
    sync when done
  • do all tempBootItems
    user actions added by doWhenBooted, in the order they were added in.

All of them allow waiting and conditions within the functions.

// Here is a variant of test_bootSequence that works as a test on 3.8.0: 
fork {
	var a, cond, b1, b2, t1, t2, oldTFunc;
	var numBootFuncs = ServerBoot.objects !? { ServerBoot.objects[s].size } ? 0;
	var numTreeFuncs = ServerTree.objects !? { ServerTree.objects[s].size } ? 0;
	var expectedList = List[ '1_Bt', '2_Bt', '3_tr', '4_Tr', '5_Tr', '6_wt', '7_do' ];
		
	// list of function names to check
	a = List[];
	// add functions to run at all boot stages
	ServerBoot.add (b1 = { |sv1| (sv1.name + ": ").post; a.add('1_Bt'.postln); }, s);
	ServerBoot.add (b2 = { |sv2| (sv2.name + ": ").post; a.add('2_Bt'.postln); }, s);
	s.tree =        { (s.name + ": ").post; 0.1.wait; a.add('3_tr'.postln); };
	ServerTree.add (t1 = { |sv| (sv.name + ": ").post; 0.1.wait; a.add('4_Tr'.postln); }, s);
	ServerTree.add (t2 = { |sv| (sv.name + ": ").post; 0.1.wait; a.add('5_Tr'.postln); }, s);
		
	s.quit;
	a = List[];
	s.waitForBoot  { ().play; s.post; a.add('6_wt'.postln); };
	s.doWhenBooted { 0.1.wait; s.post; a.add('7_do'.postln); };
	s.doWhenBooted { 2.wait; cond.unhang; };
	s.boot;
	
	cond = Condition.new;
	cond.hang;
		
	// wait for slow late tasks, just in case
	5.wait;
	
	a.postln; 
	if (a == expectedList) { 
		"*** YES, BOOT ORDER IS CORRECT. *** ".postln;
	} { 
		"*** NO, BOOT ORDER IS not correct. *** ".postln;
	};
	
	// cleanup
	ServerBoot.remove(b1, s);
	ServerBoot.remove(b2, s);
	ServerTree.remove(t1, s);
	ServerTree.remove(t2, s);
	s.quit;
}
@adcxyz

This comment has been minimized.

Show comment
Hide comment
@adcxyz

adcxyz Nov 17, 2017

Contributor

After rereading the thread now, I hope my branch is useful for trying out how that behaves.
Please feel free to write tests that break it.

Contributor

adcxyz commented Nov 17, 2017

After rereading the thread now, I hope my branch is useful for trying out how that behaves.
Please feel free to write tests that break it.

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 17, 2017

Member

Yes. Part of my thinking is to set up a clear, unambiguous dividing line between the sync / safety that the SC class library is responsible for, and the sync / safety that the user is responsible for.

Thanks for stating this. Yes, we're definitely on the same page. :)

Let's assume that there is perfect documentation of whichever design decision we make. Not necessarily saying that everyone reads it, but that the decision and contract has been clearly recorded.

The two implementation strategies we've discussed for running onComplete actions so far are:

  1. sequential: each action is run in a loop in the order it was received
  2. parallel: each action is run on a separately forked thread

I'm just going to refer to onComplete actions as "actions" for simplicity.


With regard to the current parallel implementation, I want to point out that instead of

   while { serverRunning.not } { 0.2.wait };

we could have

   serverRunningCondition.wait;

where serverRunningCondition is a Condition object whose test is set to true only when the server is running. In other words, there is a one-to-one mapping between the function of serverRunning in the current impl and the value of serverRunningCondition.test in the snippet above. Then, the wait queue inside the Condition would operate somewhat like the queue in your sequential case. Threads would definitely be set running in the order they were added via waitForBoot. The only difference is that threads could interleave if any of them yields. Or you could have a separate queue of actions that you later fork sequentially (suggested in James's comment directly above).

Since the current implementation makes no guarantees on the order in which actions begin executing, limiting it to one order by using a Condition is a non-breaking change.


Here are the pros and cons as I see them:

Parallel: Pros

  • fast: actions are allowed to make use of the time others spend sleeping or waiting for an async call to complete
  • if one action causes an error, other actions still run (as compared to the proposed sequential implementation)
  • it is the current approach, so no code is broken

Parallel: Cons

  • some users may have difficult understanding how to write correct thread-safe code
  • naive use would be prone to concurrency errors, which are often difficult to debug

Sequential: Pros

  • easy to understand
  • reduces likelihood of race conditions

Sequential: Cons

  • slower: each thread has to wait for its predecessor to finish, and the time a thread spends sleeping or waiting on an async callback is wasted. This can be alleviated by forking inside each action, but users who do not understand multithreading or the implementation of this function will not know that.
  • if one action causes an error, the rest of the actions do not run (in the proposed implementation; there are ways of fixing this of course)
  • naive use would be prone to hanging if one of the actions is an infinite waiting loop
  • breaks existing code

Another point of comparison:

Extra code needed:

  • parallel: to get working thread-safe code you will need to add in condition variables yourself, which may be difficult for users unfamiliar with multithreading. But, this can be educational and beneficial when you go on to write other multithreaded code.
  • sequential: to get parallelism you will need to use fork { }.

I don't know the relative frequencies of when and how people write thread-unsafe actions vs when they write actions that would break if run in sequence. I would certainly like to know, because the best solution will depend somewhat on that. If there's no good information, I would prefer to be conservative and stick with the approach that is not going to break existing usage.

Member

brianlheim commented Nov 17, 2017

Yes. Part of my thinking is to set up a clear, unambiguous dividing line between the sync / safety that the SC class library is responsible for, and the sync / safety that the user is responsible for.

Thanks for stating this. Yes, we're definitely on the same page. :)

Let's assume that there is perfect documentation of whichever design decision we make. Not necessarily saying that everyone reads it, but that the decision and contract has been clearly recorded.

The two implementation strategies we've discussed for running onComplete actions so far are:

  1. sequential: each action is run in a loop in the order it was received
  2. parallel: each action is run on a separately forked thread

I'm just going to refer to onComplete actions as "actions" for simplicity.


With regard to the current parallel implementation, I want to point out that instead of

   while { serverRunning.not } { 0.2.wait };

we could have

   serverRunningCondition.wait;

where serverRunningCondition is a Condition object whose test is set to true only when the server is running. In other words, there is a one-to-one mapping between the function of serverRunning in the current impl and the value of serverRunningCondition.test in the snippet above. Then, the wait queue inside the Condition would operate somewhat like the queue in your sequential case. Threads would definitely be set running in the order they were added via waitForBoot. The only difference is that threads could interleave if any of them yields. Or you could have a separate queue of actions that you later fork sequentially (suggested in James's comment directly above).

Since the current implementation makes no guarantees on the order in which actions begin executing, limiting it to one order by using a Condition is a non-breaking change.


Here are the pros and cons as I see them:

Parallel: Pros

  • fast: actions are allowed to make use of the time others spend sleeping or waiting for an async call to complete
  • if one action causes an error, other actions still run (as compared to the proposed sequential implementation)
  • it is the current approach, so no code is broken

Parallel: Cons

  • some users may have difficult understanding how to write correct thread-safe code
  • naive use would be prone to concurrency errors, which are often difficult to debug

Sequential: Pros

  • easy to understand
  • reduces likelihood of race conditions

Sequential: Cons

  • slower: each thread has to wait for its predecessor to finish, and the time a thread spends sleeping or waiting on an async callback is wasted. This can be alleviated by forking inside each action, but users who do not understand multithreading or the implementation of this function will not know that.
  • if one action causes an error, the rest of the actions do not run (in the proposed implementation; there are ways of fixing this of course)
  • naive use would be prone to hanging if one of the actions is an infinite waiting loop
  • breaks existing code

Another point of comparison:

Extra code needed:

  • parallel: to get working thread-safe code you will need to add in condition variables yourself, which may be difficult for users unfamiliar with multithreading. But, this can be educational and beneficial when you go on to write other multithreaded code.
  • sequential: to get parallelism you will need to use fork { }.

I don't know the relative frequencies of when and how people write thread-unsafe actions vs when they write actions that would break if run in sequence. I would certainly like to know, because the best solution will depend somewhat on that. If there's no good information, I would prefer to be conservative and stick with the approach that is not going to break existing usage.

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 18, 2017

Contributor

Again, I don't necessarily mind the parallel approach. I can even concede that we might be stuck with it. It strikes me as a bit of a hack from the beginning -- I can't say I'm exactly thrilled that our design choices now might be constrained by quick decisions from 14 years ago[1], but if that's the way it is, then that's the way it is. (But also, at that time, most of us weren't thinking deeply or clearly about thread sync. I certainly wasn't.)

Also, here, we are thinking about worst cases -- the hapless user who, willy-nilly, writes 10 waitForBoot blocks without thinking about consolidating them. Even with the most carefully designed system, this user is asking for trouble.

The common case is still a single waitForBoot block, and we would be totally justified to recommend in the documentation that users write one instruction to boot the server and one action to load what they need. Anything else is "clever," and the more time I spend doing this, the more I become convinced that cleverness is a gateway to stupid bugs. The old Rest implementation was clever. OSCFunc:add is clever (and broken). Users can throw a lot of post-boot actions at the system, but they probably shouldn't (meaning, if they do and they find some situations we didn't think of, we recommend that they clean up their code instead of immediately calling it a bug).

we could have serverRunningCondition.wait; where serverRunningCondition is a Condition object whose test is set to true only when the server is running.

WRT to waitForBoot actions, the relevant condition is not that the server is running -- it's that the server has completed all other initialization. I'm pressing that distinction because one of the reasons why the boot process is so messy is that we weren't precise about when things should happen.

To be honest, I catch a whiff of cleverness from the this idea. Instead of controlling evaluation order ourselves, we trust another object to do it for us. "Oh, but it already does x, y and z" ... clever. I wouldn't mind using a Condition to sync the server-init thread, but not to run the stuff.

WRT to the pros and cons of parallel vs sequential, it's largely a matter of preference. I prefer easy-to-explain and deterministic over fast. You can see that, for instance, in MixerChannel, which uses a queue. I could make it finish the creation of multiple mixers faster by running node creation in parallel, but then it would be harder to get node order right, and I need it to be stable. Your preference seems to be for speed, and for independence of multiple actions. OK... but these are not strictly objective value judgments. (EDIT: My preference is not objective, either.)

Sequential: Cons
breaks existing code

You may be exaggerating the risk here. There are a lot of cases of users writing one waitForBoot block containing wait or sync statements. The sequential approach will not break these. The actions still occur in the context of a Routine.

There may be cases of users writing multiple waitForBoot actions with the expectation that these will run as parallel routines. I have not seen this scenario come up on the mailing lists, and I've never heard any user -- not one -- demand that this is the way it must be. Again, just because users can, doesn't mean they should. (In a way, you're arguing to provide transparent support for a suboptimal usage pattern, where I think it would be better to steer users away from hidden complexity.)

But we're kind of going in circles here. So... what to do?

[1] 948738f#diff-fbcff7d470b8c2b63414aa864de95baaR185

Contributor

jamshark70 commented Nov 18, 2017

Again, I don't necessarily mind the parallel approach. I can even concede that we might be stuck with it. It strikes me as a bit of a hack from the beginning -- I can't say I'm exactly thrilled that our design choices now might be constrained by quick decisions from 14 years ago[1], but if that's the way it is, then that's the way it is. (But also, at that time, most of us weren't thinking deeply or clearly about thread sync. I certainly wasn't.)

Also, here, we are thinking about worst cases -- the hapless user who, willy-nilly, writes 10 waitForBoot blocks without thinking about consolidating them. Even with the most carefully designed system, this user is asking for trouble.

The common case is still a single waitForBoot block, and we would be totally justified to recommend in the documentation that users write one instruction to boot the server and one action to load what they need. Anything else is "clever," and the more time I spend doing this, the more I become convinced that cleverness is a gateway to stupid bugs. The old Rest implementation was clever. OSCFunc:add is clever (and broken). Users can throw a lot of post-boot actions at the system, but they probably shouldn't (meaning, if they do and they find some situations we didn't think of, we recommend that they clean up their code instead of immediately calling it a bug).

we could have serverRunningCondition.wait; where serverRunningCondition is a Condition object whose test is set to true only when the server is running.

WRT to waitForBoot actions, the relevant condition is not that the server is running -- it's that the server has completed all other initialization. I'm pressing that distinction because one of the reasons why the boot process is so messy is that we weren't precise about when things should happen.

To be honest, I catch a whiff of cleverness from the this idea. Instead of controlling evaluation order ourselves, we trust another object to do it for us. "Oh, but it already does x, y and z" ... clever. I wouldn't mind using a Condition to sync the server-init thread, but not to run the stuff.

WRT to the pros and cons of parallel vs sequential, it's largely a matter of preference. I prefer easy-to-explain and deterministic over fast. You can see that, for instance, in MixerChannel, which uses a queue. I could make it finish the creation of multiple mixers faster by running node creation in parallel, but then it would be harder to get node order right, and I need it to be stable. Your preference seems to be for speed, and for independence of multiple actions. OK... but these are not strictly objective value judgments. (EDIT: My preference is not objective, either.)

Sequential: Cons
breaks existing code

You may be exaggerating the risk here. There are a lot of cases of users writing one waitForBoot block containing wait or sync statements. The sequential approach will not break these. The actions still occur in the context of a Routine.

There may be cases of users writing multiple waitForBoot actions with the expectation that these will run as parallel routines. I have not seen this scenario come up on the mailing lists, and I've never heard any user -- not one -- demand that this is the way it must be. Again, just because users can, doesn't mean they should. (In a way, you're arguing to provide transparent support for a suboptimal usage pattern, where I think it would be better to steer users away from hidden complexity.)

But we're kind of going in circles here. So... what to do?

[1] 948738f#diff-fbcff7d470b8c2b63414aa864de95baaR185

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon Nov 18, 2017

Member

I am thinking in particular about the cons of sequential scheduling here.

slower: each thread has to wait for its predecessor to finish, and the time a thread spends sleeping or waiting on an async callback is wasted.

For our use case, that is booting a server, does that matter? The only case I could come up with would be large and independent synth def or file reading that happens after boot. But doesn't the server also put these things in a queue? If yes, then the only extra delay would be the network handshake time. Slower here means also more deterministic, which is really so much needed.

if one action causes an error, the rest of the actions do not run

I would have counted this as an advantage. Do you have an example when you would want to have that?

naive use would be prone to hanging if one of the actions is an infinite waiting loop

This is true. But I've never seen anything like this in use. An action function is normally not thought of having a wait inside. We could even find a way to prevent code that calls wait or sync inside such a function, if this is important.

breaks existing code

Apart from the loop case, is there anything else that would break?

Sequentialism in Parallelism

It is much easier to build parallelism from sequential order than the other way round (this is why Routines are not system threads).

The main difficulty I see with the parallel approach as opposed to sequential scheduling is that it is cumbersome to enforce execution order. Of course, one can always do it by using conditions, but so far, few of us seem to be doing this.

If we decide that we want parallelism, we should really work on improving the documentation and interface of the conditions (adding promises etc.).

Member

telephon commented Nov 18, 2017

I am thinking in particular about the cons of sequential scheduling here.

slower: each thread has to wait for its predecessor to finish, and the time a thread spends sleeping or waiting on an async callback is wasted.

For our use case, that is booting a server, does that matter? The only case I could come up with would be large and independent synth def or file reading that happens after boot. But doesn't the server also put these things in a queue? If yes, then the only extra delay would be the network handshake time. Slower here means also more deterministic, which is really so much needed.

if one action causes an error, the rest of the actions do not run

I would have counted this as an advantage. Do you have an example when you would want to have that?

naive use would be prone to hanging if one of the actions is an infinite waiting loop

This is true. But I've never seen anything like this in use. An action function is normally not thought of having a wait inside. We could even find a way to prevent code that calls wait or sync inside such a function, if this is important.

breaks existing code

Apart from the loop case, is there anything else that would break?

Sequentialism in Parallelism

It is much easier to build parallelism from sequential order than the other way round (this is why Routines are not system threads).

The main difficulty I see with the parallel approach as opposed to sequential scheduling is that it is cumbersome to enforce execution order. Of course, one can always do it by using conditions, but so far, few of us seem to be doing this.

If we decide that we want parallelism, we should really work on improving the documentation and interface of the conditions (adding promises etc.).

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon Nov 18, 2017

Member

if one action causes an error, the rest of the actions do not run

If we wanted to avoid this, we could wrap all actions in a protect, something like:

protect {
     function.value
} {
     thisThread.clock.sched(0, thisThread);
}

(untested)

Member

telephon commented Nov 18, 2017

if one action causes an error, the rest of the actions do not run

If we wanted to avoid this, we could wrap all actions in a protect, something like:

protect {
     function.value
} {
     thisThread.clock.sched(0, thisThread);
}

(untested)

@adcxyz

This comment has been minimized.

Show comment
Hide comment
@adcxyz

adcxyz Nov 18, 2017

Contributor

@brianlheim - thanks for the clear exposition of the two approaches!

Do you write your own setups in concurrent style, and can you show examples?
Do you have testable examples of larger setups where you could measure speed differences?
I would be curious to see how a well-designed hand-written parallel load script
with forks and conditions should look in your opinion.
That would be a great sketch for how a ConcurrentBoot class would work.

Personally, I have done my own setup scripts strictly sequential for years, e.g.:

// offline stuff first, then
fork {
	s.quit; 
	1.wait; 
	s.waitForBoot({ 
		0.2.wait; 
		"// scripts for buffers etc".postln; 
		0.2.wait; 
		s.sync;
		"// load Ndefs, Pdefs, Tdefs, FX etc".postln;
		0.2.wait; 
		s.sync;
		"// controller setups, guis".postln;
	});
};

completely predictable, occasional breathing time added, etc.

I expect most users write one boot sequence that loads all they need for the project at hand:
be it for an installation, a rendered piece, a 3hr live set, a standalone app, etc.
For that, the top priority is reliability, simplicity, predictability, code readability:
everything should load predictably every time;
when something fails, chances are something else will need it, so stop and inform;
if code is easy to follow, we know where we are and can restart from there.
Maximally reliable final loaded state seems worth a tradeoff in loading speed.

An important time aspect to consider is artist coding time ... I think the default canonical style for setups in SC should advise people to do things (code, load script files) in the clearest order they can think of, to make sure every bit of infrastructure is already there when the next bit of code needs it.
This makes it easy to fix any problems, as they are as easy as possible to reason about.

For these reasons, I prefer simplifying the boot sequence as the choice for SC's default behavior.
If the speedups shown by sketches using concurrency loading are promising, why not implement a ConcurrentBoot class that handles the forks and conditions elegantly, and use that where its benefits outweigh those of sequential style?
2c adc

Contributor

adcxyz commented Nov 18, 2017

@brianlheim - thanks for the clear exposition of the two approaches!

Do you write your own setups in concurrent style, and can you show examples?
Do you have testable examples of larger setups where you could measure speed differences?
I would be curious to see how a well-designed hand-written parallel load script
with forks and conditions should look in your opinion.
That would be a great sketch for how a ConcurrentBoot class would work.

Personally, I have done my own setup scripts strictly sequential for years, e.g.:

// offline stuff first, then
fork {
	s.quit; 
	1.wait; 
	s.waitForBoot({ 
		0.2.wait; 
		"// scripts for buffers etc".postln; 
		0.2.wait; 
		s.sync;
		"// load Ndefs, Pdefs, Tdefs, FX etc".postln;
		0.2.wait; 
		s.sync;
		"// controller setups, guis".postln;
	});
};

completely predictable, occasional breathing time added, etc.

I expect most users write one boot sequence that loads all they need for the project at hand:
be it for an installation, a rendered piece, a 3hr live set, a standalone app, etc.
For that, the top priority is reliability, simplicity, predictability, code readability:
everything should load predictably every time;
when something fails, chances are something else will need it, so stop and inform;
if code is easy to follow, we know where we are and can restart from there.
Maximally reliable final loaded state seems worth a tradeoff in loading speed.

An important time aspect to consider is artist coding time ... I think the default canonical style for setups in SC should advise people to do things (code, load script files) in the clearest order they can think of, to make sure every bit of infrastructure is already there when the next bit of code needs it.
This makes it easy to fix any problems, as they are as easy as possible to reason about.

For these reasons, I prefer simplifying the boot sequence as the choice for SC's default behavior.
If the speedups shown by sketches using concurrency loading are promising, why not implement a ConcurrentBoot class that handles the forks and conditions elegantly, and use that where its benefits outweigh those of sequential style?
2c adc

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 18, 2017

Contributor

slower: each thread has to wait for its predecessor to finish, and the time a thread spends sleeping or waiting on an async callback is wasted.
For our use case, that is booting a server, does that matter? The only case I could come up with would be large and independent synth def or file reading that happens after boot.

The case is: a user calls waitForBoot 5 times, and each function has 2 1.wait statements. Run sequentially, the piece will be loaded in 10 seconds. Run in parallel, it drops to 2 seconds.

But that begs the question, why is the user calling waitForBoot so many times?

if one action causes an error, the rest of the actions do not run
I would have counted this as an advantage.

It didn't occur to me until now that there's a little inconsistency here: on the one hand, the user should be sophisticated enough to sync up dependent actions across multiple threads, but when it comes to errors, a later function is expected to succeed even if prior ones failed (that is, no dependencies).

It is much easier to build parallelism from sequential order than the other way round.

Thank you, this is what I've been trying to say all along.

Contributor

jamshark70 commented Nov 18, 2017

slower: each thread has to wait for its predecessor to finish, and the time a thread spends sleeping or waiting on an async callback is wasted.
For our use case, that is booting a server, does that matter? The only case I could come up with would be large and independent synth def or file reading that happens after boot.

The case is: a user calls waitForBoot 5 times, and each function has 2 1.wait statements. Run sequentially, the piece will be loaded in 10 seconds. Run in parallel, it drops to 2 seconds.

But that begs the question, why is the user calling waitForBoot so many times?

if one action causes an error, the rest of the actions do not run
I would have counted this as an advantage.

It didn't occur to me until now that there's a little inconsistency here: on the one hand, the user should be sophisticated enough to sync up dependent actions across multiple threads, but when it comes to errors, a later function is expected to succeed even if prior ones failed (that is, no dependencies).

It is much easier to build parallelism from sequential order than the other way round.

Thank you, this is what I've been trying to say all along.

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 18, 2017

Member

(James) Your preference seems to be for speed, and for independence of multiple actions. OK... but these are not strictly objective value judgments.

My overall preference is to not break existing code because of anyone's personal preference. My main purpose in listing the pros and cons was to make everyone aware of the consequences of this decision. In general, yes, I prefer speed first and then getting the safety I pay for, rather than the other way around, which is probably why I got thinking about this in the first place, but there is more to it than that.

(Julian) An action function is normally not thought of having a wait inside.

Alberto's comment above includes what is typical of his setups, and it has several calls to wait in it.

(Julian) Apart from the loop case, is there anything else that would break?

Anything that depends on the actions being run in parallel or finishing in a finite amount of time. That is a large (but relatively rare) range of conditions. Anything where action execution time has now doubled or n-tupled because of this change could be considered "soft" broken.

(Alberto) I expect most users write one boot sequence that loads all they need for the project at hand:
be it for an installation, a rendered piece, a 3hr live set, a standalone app, etc.
For that, the top priority is reliability, simplicity, predictability, code readability:

If there is one boot sequence (i.e. one waitForBoot call), the choice of a sequential or parallel execution pattern makes absolutely no difference. Both are (practically) equally reliable, simple, predictable, and readable.

(Alberto) Do you write your own setups in concurrent style, and can you show examples?
I would be curious to see how a well-designed hand-written parallel load script
with forks and conditions should look in your opinion.

No, can anyone here show an example of a setup with multiple calls to waitForBoot? I have been curious this whole time to see a script "from the wild" that is representative of the kinds of issues users are running into with parallelism. Without one, IMO this is starting to seem like trying to solve a problem no one asked to be solved.

(James) But we're kind of going in circles here. So... what to do?

(Alberto) I expect most users write one boot sequence that loads all they need for the project at hand

(Alberto) Personally, I have done my own setup scripts strictly sequential for years,

(James) why is the user calling waitForBoot so many times?

(James) we would be totally justified to recommend in the documentation that users write one instruction to boot the server and one action to load what they need

It seems from this thread that there is really no good reason to choose one over the other because for a majority of people (anyway, the majority of the small minority of people whose code you see on the mailing list), it simply doesn't matter. They're only making use of one thread. Nobody is getting tripped up by race conditions, and nobody is getting frustrated with blocking loops. I started off this discussion thinking that this design choice was an attempt to solve some common problem, but I've seen no evidence of that. So, I think the argument from responsible maintenance is what I'm going to continue to fall back to. If you decide you are OK potentially mucking it up for some users in this case, that's fine by me.

Member

brianlheim commented Nov 18, 2017

(James) Your preference seems to be for speed, and for independence of multiple actions. OK... but these are not strictly objective value judgments.

My overall preference is to not break existing code because of anyone's personal preference. My main purpose in listing the pros and cons was to make everyone aware of the consequences of this decision. In general, yes, I prefer speed first and then getting the safety I pay for, rather than the other way around, which is probably why I got thinking about this in the first place, but there is more to it than that.

(Julian) An action function is normally not thought of having a wait inside.

Alberto's comment above includes what is typical of his setups, and it has several calls to wait in it.

(Julian) Apart from the loop case, is there anything else that would break?

Anything that depends on the actions being run in parallel or finishing in a finite amount of time. That is a large (but relatively rare) range of conditions. Anything where action execution time has now doubled or n-tupled because of this change could be considered "soft" broken.

(Alberto) I expect most users write one boot sequence that loads all they need for the project at hand:
be it for an installation, a rendered piece, a 3hr live set, a standalone app, etc.
For that, the top priority is reliability, simplicity, predictability, code readability:

If there is one boot sequence (i.e. one waitForBoot call), the choice of a sequential or parallel execution pattern makes absolutely no difference. Both are (practically) equally reliable, simple, predictable, and readable.

(Alberto) Do you write your own setups in concurrent style, and can you show examples?
I would be curious to see how a well-designed hand-written parallel load script
with forks and conditions should look in your opinion.

No, can anyone here show an example of a setup with multiple calls to waitForBoot? I have been curious this whole time to see a script "from the wild" that is representative of the kinds of issues users are running into with parallelism. Without one, IMO this is starting to seem like trying to solve a problem no one asked to be solved.

(James) But we're kind of going in circles here. So... what to do?

(Alberto) I expect most users write one boot sequence that loads all they need for the project at hand

(Alberto) Personally, I have done my own setup scripts strictly sequential for years,

(James) why is the user calling waitForBoot so many times?

(James) we would be totally justified to recommend in the documentation that users write one instruction to boot the server and one action to load what they need

It seems from this thread that there is really no good reason to choose one over the other because for a majority of people (anyway, the majority of the small minority of people whose code you see on the mailing list), it simply doesn't matter. They're only making use of one thread. Nobody is getting tripped up by race conditions, and nobody is getting frustrated with blocking loops. I started off this discussion thinking that this design choice was an attempt to solve some common problem, but I've seen no evidence of that. So, I think the argument from responsible maintenance is what I'm going to continue to fall back to. If you decide you are OK potentially mucking it up for some users in this case, that's fine by me.

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon Nov 18, 2017

Member

All of these actions would now be happening after initTree, like ServerTree, wouldn't they? The boot logic itself is internal, with the only exception to ServerBoot. I think at least the internal order should happen in one single routine.

Member

telephon commented Nov 18, 2017

All of these actions would now be happening after initTree, like ServerTree, wouldn't they? The boot logic itself is internal, with the only exception to ServerBoot. I think at least the internal order should happen in one single routine.

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 18, 2017

Member

I think at least the internal order should happen in one single routine.

Yes, definitely, I think that's a great idea!

Member

brianlheim commented Nov 18, 2017

I think at least the internal order should happen in one single routine.

Yes, definitely, I think that's a great idea!

@telephon

This comment has been minimized.

Show comment
Hide comment
@telephon

telephon Nov 18, 2017

Member

… and then, we could slowly shift, if we wanted, the server tree / waitForBoot actions, either deprecating the interface, or adding something to it. But that could then wait (sic) for a future version.

Member

telephon commented Nov 18, 2017

… and then, we could slowly shift, if we wanted, the server tree / waitForBoot actions, either deprecating the interface, or adding something to it. But that could then wait (sic) for a future version.

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 18, 2017

Contributor

I won't object further. I do find the argument for backward compatibility to be compelling. I don't like it, but it's compelling. (IMO it's the only argument in favor of parallel waitForBoot actions that is compelling.)

I want to be on the record as saying that we have implicit parallelism here because none of us knew what we were doing back in 2003. SC2 didn't require this kind of control over asynchronous init. As a result, pretty much the only dev who understood thread sync was James McCartney, and if he contributed code, he would contractually have to give it to Apple, so we were on our own. Routine { while { not_ready } { wait }; continue } is exactly what you would do if you're suddenly confronted with asynchronicity and had no knowledge of semaphores or mutexes. (If it looks like I'm pointing fingers, I absolutely include myself in this. I had no clue about sync, 14 years ago.) That is, waitForBoot is parallel only by accident. Now, we are constrained, by yesterday's lack of knowledge, to carry this accident forward indefinitely. I can accept that constraint, but I don't have to accept that it's technically ideal.

If backward compatibility is the primary consideration, then I have to rethink my objection to the Condition approach: because backward compatibility mandates that doWhenBooted return a Routine. I need to think about that a bit more, though.

Contributor

jamshark70 commented Nov 18, 2017

I won't object further. I do find the argument for backward compatibility to be compelling. I don't like it, but it's compelling. (IMO it's the only argument in favor of parallel waitForBoot actions that is compelling.)

I want to be on the record as saying that we have implicit parallelism here because none of us knew what we were doing back in 2003. SC2 didn't require this kind of control over asynchronous init. As a result, pretty much the only dev who understood thread sync was James McCartney, and if he contributed code, he would contractually have to give it to Apple, so we were on our own. Routine { while { not_ready } { wait }; continue } is exactly what you would do if you're suddenly confronted with asynchronicity and had no knowledge of semaphores or mutexes. (If it looks like I'm pointing fingers, I absolutely include myself in this. I had no clue about sync, 14 years ago.) That is, waitForBoot is parallel only by accident. Now, we are constrained, by yesterday's lack of knowledge, to carry this accident forward indefinitely. I can accept that constraint, but I don't have to accept that it's technically ideal.

If backward compatibility is the primary consideration, then I have to rethink my objection to the Condition approach: because backward compatibility mandates that doWhenBooted return a Routine. I need to think about that a bit more, though.

@brianlheim

This comment has been minimized.

Show comment
Hide comment
@brianlheim

brianlheim Nov 18, 2017

Member

(IMO it's the only argument in favor of parallel waitForBoot actions that is compelling.)

Honestly, at this point, I pretty much agree. I think yall have presented a very convincing case and I'm really glad we got the opportunity to discuss it.

Routine { while { not_ready } { wait }; continue } is exactly what you would do if you're suddenly confronted with asynchronicity and had no knowledge of semaphores or mutexes. (If it looks like I'm pointing fingers, I absolutely include myself in this. I had no clue about sync, 14 years ago.)

To be fair I also had no clue until about 5 months ago. This is tricky stuff!

Member

brianlheim commented Nov 18, 2017

(IMO it's the only argument in favor of parallel waitForBoot actions that is compelling.)

Honestly, at this point, I pretty much agree. I think yall have presented a very convincing case and I'm really glad we got the opportunity to discuss it.

Routine { while { not_ready } { wait }; continue } is exactly what you would do if you're suddenly confronted with asynchronicity and had no knowledge of semaphores or mutexes. (If it looks like I'm pointing fingers, I absolutely include myself in this. I had no clue about sync, 14 years ago.)

To be fair I also had no clue until about 5 months ago. This is tricky stuff!

@adcxyz

This comment has been minimized.

Show comment
Hide comment
@adcxyz

adcxyz Nov 19, 2017

Contributor

Dear all, just for my understanding, we agree now that:

  • booting the server app until notification /clientID should be in one single Routine
  • prRunBootTask should then do s.tree, ServerTree, only then set serverRunning to true
  • waitForBoot and doWhenBooted should stay parallel, i.e. each one starts a separate routine
  • these routines see serverRunning true AFTER prRunBootTask / ServerTree have finished.

Yes? If so, then I will adjust #3299 accordingly.

  • Someone should then add documentation that gives users the gist of this discussion...

Questions (assuming yes):

  • what to do about doWhenBooted / waitForBoot args limit and onFailure?
    ATM the tasks wait forever if the server never boots, or until cmd-period ...
    OTOH, if you call doWhenBooted early, then the setup takes more than 20 seconds
    before the server is finally booted, the task will timeout and do onFailure.
    Do we consider that user error? (default limit could be higher, or even inf)

  • Should I leave some form of tempBootTasks in for future support of sequential boot tasks?
    could be something like server.addToBootSetup ({ ... });
    Maybe one would want a choice of temp or permanent:
    server.addToBootSetup ({ ... }, once: true);

Contributor

adcxyz commented Nov 19, 2017

Dear all, just for my understanding, we agree now that:

  • booting the server app until notification /clientID should be in one single Routine
  • prRunBootTask should then do s.tree, ServerTree, only then set serverRunning to true
  • waitForBoot and doWhenBooted should stay parallel, i.e. each one starts a separate routine
  • these routines see serverRunning true AFTER prRunBootTask / ServerTree have finished.

Yes? If so, then I will adjust #3299 accordingly.

  • Someone should then add documentation that gives users the gist of this discussion...

Questions (assuming yes):

  • what to do about doWhenBooted / waitForBoot args limit and onFailure?
    ATM the tasks wait forever if the server never boots, or until cmd-period ...
    OTOH, if you call doWhenBooted early, then the setup takes more than 20 seconds
    before the server is finally booted, the task will timeout and do onFailure.
    Do we consider that user error? (default limit could be higher, or even inf)

  • Should I leave some form of tempBootTasks in for future support of sequential boot tasks?
    could be something like server.addToBootSetup ({ ... });
    Maybe one would want a choice of temp or permanent:
    server.addToBootSetup ({ ... }, once: true);

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 19, 2017

Contributor

what to do about doWhenBooted / waitForBoot args limit and onFailure? ATM the tasks wait forever if the server never boots, or until cmd-period

This one is on my mind, but I haven't thought it all the way through yet.

I don't think we can maintain independent timeouts per waitForBoot (WFB). Either the server comes up or it doesn't. If the server process dies before the timeout, we should stop the boot process, and then none of the WFB actions fire anyway (and, IMO, they should be cleared at that point)... so independent timeouts are irrelevant in that case.

Or the server process doesn't die, but hangs. Again, the boot init process will probably get stuck before the user actions. Using condition.wait(timeout) may help here.

In hindsight, a user's WFB action is not the right place to determine whether the server started successfully or not... so it isn't the right place to set the timeout duration either.

Contributor

jamshark70 commented Nov 19, 2017

what to do about doWhenBooted / waitForBoot args limit and onFailure? ATM the tasks wait forever if the server never boots, or until cmd-period

This one is on my mind, but I haven't thought it all the way through yet.

I don't think we can maintain independent timeouts per waitForBoot (WFB). Either the server comes up or it doesn't. If the server process dies before the timeout, we should stop the boot process, and then none of the WFB actions fire anyway (and, IMO, they should be cleared at that point)... so independent timeouts are irrelevant in that case.

Or the server process doesn't die, but hangs. Again, the boot init process will probably get stuck before the user actions. Using condition.wait(timeout) may help here.

In hindsight, a user's WFB action is not the right place to determine whether the server started successfully or not... so it isn't the right place to set the timeout duration either.

@adcxyz

This comment has been minimized.

Show comment
Hide comment
@adcxyz

adcxyz Nov 19, 2017

Contributor

In hindsight, a user's WFB action is not the right place to determine whether the server started successfully or not... so it isn't the right place to set the timeout duration either.

TTBOMK, booting the server always goes through server:boot,
so any onFailure action(s) and timeouts should best be in that call only, right?
Then waitForBoot could keep their limit and onFailure args, because it can pass them to boot,
and doWhenBooted could ignore them, and inform users about that when the args are used.
would that be the best way to handle this?

Contributor

adcxyz commented Nov 19, 2017

In hindsight, a user's WFB action is not the right place to determine whether the server started successfully or not... so it isn't the right place to set the timeout duration either.

TTBOMK, booting the server always goes through server:boot,
so any onFailure action(s) and timeouts should best be in that call only, right?
Then waitForBoot could keep their limit and onFailure args, because it can pass them to boot,
and doWhenBooted could ignore them, and inform users about that when the args are used.
would that be the best way to handle this?

@jamshark70

This comment has been minimized.

Show comment
Hide comment
@jamshark70

jamshark70 Nov 20, 2017

Contributor

Then waitForBoot could keep their limit and onFailure args, because it can pass them to boot,
and doWhenBooted could ignore them, and inform users about that when the args are used.

Hm, yes, that's pretty much what I was thinking. I don't think we should deprecate arguments now (maybe in the future).

How about this:

  • Change the default value for limit to nil (in the argument declarations for waitForBoot and doWhenBooted).

  • First waitForBoot call sets the timeout. Second waitForBoot call (or really, any call to waitForBoot where the server status is not \off): if it gets a value for limit that disagrees with the previously-set timeout, print a warning (but continue).

  • doWhenBooted: If the user provides a limit, print a warning and ignore the value.

  • Make it clear in the documentation that the timeout happens in the main server boot process, but is set here for convenience and backward compatibility.

So these would be OK:

(
s.waitForBoot({ ... }, limit: 20);

s.waitForBoot({ ... });  // or doWhenBooted
)

(
s.waitForBoot({ ... }, limit: 20);

s.waitForBoot({ ... }, limit: 20);
)

... while this would print a warning (and the second call would be subject to the limit of 20).

(
s.waitForBoot({ ... }, limit: 20);

s.waitForBoot({ ... }, limit: 50);
)
Contributor

jamshark70 commented Nov 20, 2017

Then waitForBoot could keep their limit and onFailure args, because it can pass them to boot,
and doWhenBooted could ignore them, and inform users about that when the args are used.

Hm, yes, that's pretty much what I was thinking. I don't think we should deprecate arguments now (maybe in the future).

How about this:

  • Change the default value for limit to nil (in the argument declarations for waitForBoot and doWhenBooted).

  • First waitForBoot call sets the timeout. Second waitForBoot call (or really, any call to waitForBoot where the server status is not \off): if it gets a value for limit that disagrees with the previously-set timeout, print a warning (but continue).

  • doWhenBooted: If the user provides a limit, print a warning and ignore the value.

  • Make it clear in the documentation that the timeout happens in the main server boot process, but is set here for convenience and backward compatibility.

So these would be OK:

(
s.waitForBoot({ ... }, limit: 20);

s.waitForBoot({ ... });  // or doWhenBooted
)

(
s.waitForBoot({ ... }, limit: 20);

s.waitForBoot({ ... }, limit: 20);
)

... while this would print a warning (and the second call would be subject to the limit of 20).

(
s.waitForBoot({ ... }, limit: 20);

s.waitForBoot({ ... }, limit: 50);
)
@adcxyz

This comment has been minimized.

Show comment
Hide comment
@adcxyz

adcxyz Nov 20, 2017

Contributor

yes, sounds like a good way to handle backwards compatibility.

In the meantime, I worked a bit more on your peoposal that all boot steps be in a single routine, and I have that working now:
A new method bootAlt, as an alternative for Server:boot, which features :

  • all boot steps happen in a single task (with forks, conditions and timeouts)
  • any running scsynths can be recovered or rebooted flexibly (recover flag)
  • posts more info on all steps and the times they take (useful for debugging, can go when done)
  • has arguments for onComplete and timeout for full flexibility
  • manual test suite in ... /testsuite/sclang/sclang_bootAltDemo_Tests.scd

// not finished / caveats

  • assumes startAliveThread is always true (which it probably always should be)
  • keeping busses and buffers in recover mode not done yet
  • boot has the same mechanism done quickly, needs more polishing - I wanted to get bootAlt presentable for discussion first.
  • states \isOff etc currently used in parallel to flags like serverBooting, serverRunning etc,
    states could eventually fully replace these for more clarity.
  • there is a lot of posting now mainly for dev/debug, to make sure we understand when what actually happens. much of this can go again when ready.

Curious to hear what you think!

Contributor

adcxyz commented Nov 20, 2017

yes, sounds like a good way to handle backwards compatibility.

In the meantime, I worked a bit more on your peoposal that all boot steps be in a single routine, and I have that working now:
A new method bootAlt, as an alternative for Server:boot, which features :

  • all boot steps happen in a single task (with forks, conditions and timeouts)
  • any running scsynths can be recovered or rebooted flexibly (recover flag)
  • posts more info on all steps and the times they take (useful for debugging, can go when done)
  • has arguments for onComplete and timeout for full flexibility
  • manual test suite in ... /testsuite/sclang/sclang_bootAltDemo_Tests.scd

// not finished / caveats

  • assumes startAliveThread is always true (which it probably always should be)
  • keeping busses and buffers in recover mode not done yet
  • boot has the same mechanism done quickly, needs more polishing - I wanted to get bootAlt presentable for discussion first.
  • states \isOff etc currently used in parallel to flags like serverBooting, serverRunning etc,
    states could eventually fully replace these for more clarity.
  • there is a lot of posting now mainly for dev/debug, to make sure we understand when what actually happens. much of this can go again when ready.

Curious to hear what you think!

@vivid-synth

This comment has been minimized.

Show comment
Hide comment
@vivid-synth

vivid-synth Nov 28, 2017

Member

Just to be proactive:

I'm staying back from venturing into this because you're all capable people and this (from my reading) is an sclang issue. That said, can you please notify loudly if the topic of changing server behavior (scsynth and supernova, not the Server class) even gets broached?

Muchas gracias!

Member

vivid-synth commented Nov 28, 2017

Just to be proactive:

I'm staying back from venturing into this because you're all capable people and this (from my reading) is an sclang issue. That said, can you please notify loudly if the topic of changing server behavior (scsynth and supernova, not the Server class) even gets broached?

Muchas gracias!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment