Roll20 uses cookies to improve your experience on our site. Cookies enable you to enjoy certain features, social sharing functionality, and tailor message and display ads to your interests on our site and others. They also help us understand how our site is being used. By continuing to use our site, you consent to our use of cookies. Update your cookie preferences .
×
Create a free account
This post has been closed. You can still view previous posts, but you can't post any new replies.

New API Update: Sheet Worker Support in API Scripts, Speed Improvements

Oooo startsWith does indeed look good. Will make things a little easier.
1489152348
The Aaron
Pro
API Scripter
Async/await anyone?  =D
The Aaron said: Async/await anyone?  =D What does that mean?
1489154335
The Aaron
Pro
API Scripter
It's the next iteration of Promises and asynchronous programming. I'll PM you some examples.
Lucian said: Is there any way that this could be made fundamentally deterministic? As you yourself pointed out, fixing performance problems is all very well, but really the priority needs to be ensuring that the callback happens when it's supposed to. At some point, somewhere, there's going to be a big campaign in use when the API server is having a bad day, and we'll creep back over the timeout limit again even with everything working the way it ought to... and then anything that's relying on the ordering implied by the callback is going to break again... Yes I agree that some work still needs to be done. However, I did have a quick question. When I added a bunch of logging to figure out the issue, I noticed that in-between your sheet worker code there were a lot of calls to findObjs({_type: "graphic"}). Since I don't *think* that's something that a Sheet Worker script would be doing, does that mean that you are mixing setWithWorker() calls with other non-sheet worker API calls like findObjs() or createObj()? If so, I think this is what was happening: onSheetWorkerCompleted() setWithWorker() (sheet worker does a bunch of work here in the worker thread) findObjs() kicks off in the main thread (while findObjs() is running the main thread, the sheet worker thread finishes) findObjs() returns after 1 second, freeing up the main thread onSheetWorkerCompleted()  runs on the main thread because the sheet worker thread queue is now empty setWithWorker() <-- this would happen post-completed-callback. Like in that case, even if I made onSheetWorkerCompleted "deterministic", it would be correct to say that it should fire its callback after the findObjs() because at that point the sheet worker queue is empty of work. It seems like what we would actually want is callback that waits until *all* processing is done both inside the sheet worker thread and on the main API thread? If that's not right let me know, just trying to figure out the correct solution for this need.
1489160011

Edited 1489160112
Lucian
Pro
API Scripter
EDIT: Crossed messages. Will reply to your new message.
1489163145

Edited 1489163280
Lucian
Pro
API Scripter
Ok, so this is really weird. I have just looked through my code quite carefully and there are only two places that call findObjs with type:graphic. One of them is in a piece of code that responds to chat messages. Since no chat messages have fired by this point, there's no way that code could have triggered. Even if it had, it's behind a guard that would only respond to messages with a roll template field {{fx=SOMETHING}}. So I'm pretty sure it's not that. The other piece of code is something that responds to changes in attributes that are configured to be connected to token bars. First of all, this should never be fired because it's an event handler and shouldn't be triggered by other API changes. Secondly, there's a guard that only responds to attributes that match a particular list of names. I've also checked that there's nothing in any of the other scripts installed in that campaign, and I can't see anything. I'm about 98% sure that I'm not responsible for these calls! I don't see how the Sheetworkers could do this either. Is there some way that you can catch these and throw an exception to see where they're coming from? Furthermore, in the case that we were looking at, I set exactly one attribute, and wait for the completion callback on that. I don't do anything else before that callback returns. This is on campaign startup, and the whole of the rest of the script startup is inside the sheetworker callback. So even if this could, in theory, be a problem in other situations, I don't think it applies here. As I said in the message that crossed with yours - I'm still seeing a limited version of the undesirable behaviour in larger campaigns that users have allowed me test with. There aren't the same huge delays that I saw in my artificial testcase (now fixed) but there are occasionally gaps that are just long enough to let the timeout expire before things have completed. To be 100% clear, when I do setWithWorker, does that immediately start stuff off in the worker thread, even before the main thread returns? Although I don't believe it's relevant to the problem in hand, I do think there are potential issues with this, in that it becomes impossible to set multiple attributes in an atomic operation, which could easily leads to all manner of unexpected/undesirable behaviour. Given that we only get to register one, general callback, I'd ideally like to see the behaviour be more like this: [main thread] onSheetworkerCompleted registered [main thread] attr1.setWithWorker - value changes but sheet worker not executed yet [main thread] attr2.setWithWorker - value changes but sheet worker not executed yet [main thread] returns [main thread] look at list of changed attibutes, work out which workers need to fire [sheet worker thread] run lots of sheetworker stuff (sheetworkers can rely on all attribute values being set as main thread intended) [main thread] do some other expensive things in the meantime [sheet worker thread] finishes [main thread] expensive thing finishes [main thread] call onSheetWorkerCompleted callback I take it as read that if I start doing stuff that modifies attributes (with or without workers) in the main thread after it has returned at (4) but before the callback at (9) then the behaviour is unpredictable. As far as I am aware, I don't do this anywhere, precisely because I'm trying to keep things neat and deterministic. I'm using Promises to ensure that all my setWithWorker stuff is serialised. The only thing I don't have control over at the moment is the fact that I do sometimes have to set multiple attributes in one batch, but I tend to do that in a single loop all in one go - and as I've just outlined, I'd hope that all those values get set at once before any workers start to fire, otherwise the only way I'm going to get reliable behaviour is to serialise every attribute modification in the script, which I think is going to be bad news for everybody!
1489180988

Edited 1489181469
Riley D.
Roll20 Team
Well, it's more complicated than that, because the Sheet Workers are themselves calling back to the main thread to do things like fetch or set attributes. Assume each line break is the end of a stack where main thread will move on to something else, and where I use -> are places that the main thread is "locked" so it will be guaranteed to happen immediately in the same call stack. If I used the same number twice, it indicates that there are two separate threads operating at the same time, so these things will be happening simultaneously: 1. [main] atrr1.setWithWorker() -> set() (values changed) -> generate message to send to sheet workers 2. [sw] receive message, trigger callbacks, callback calls getAttrs() -> send message to [main] requesting attribute values 2. [main] attr2.setWithWorker() -> set() -> generate message to send to sheet workers 3. [sw] receive message from attr2.setWithWorker(), trigger callback, callback calls getAttrs() -> send message to [main] requesting attribute values ...at this point assume you had nothing else you had called on the main thread in your script. 4. [main] respond to getAttrs() call via a message event from [sw], do findObj(), send message back to [sw] with results from findObj() 5. [sw] receive response to first getAttrs() call, further processing, callback calls setAttrs() 5. [main] respond to second getAttrs() call via a message event from [sw], do findObj(), send message back to [sw] with results from findObj() 6. [main] respond to setAttrs() call from [sw], call setWithWorker() -> set() -> (if silent, stop) -> tell [sw] we've set attributes 6. [sw] receive response to second getAttrs() call, further processing, callback calls setAttrs() 7. [main] respond to second setAttrs() call from [sw], call setWithWorker() -> set() -> (if silent, stop) -> tell [sw] we've set attributes 7. [sw] respond to notification attributes were set from first setAttrs() call ...etc. So, to answer your questions based on that: Lucian said: To be 100% clear, when I do setWithWorker, does that immediately start stuff off in the worker thread, even before the main thread returns? Although I don't believe it's relevant to the problem in hand, I do think there are potential issues with this, in that it becomes impossible to set multiple attributes in an atomic operation, which could easily leads to all manner of unexpected/undesirable behaviour.  The main thread would process the setWithWorker(), which would set the actual value immediately. Then it would generate a message to notify the sheetworker thread that the values have changed. It would send that message. Then it would move immediately onto it's next instruction, which is to do the second setWithWorker() call. So all setWithWorker() calls in a row on the main thread would process prior to the first execution of the sheet worker being able to do anything, since the sheetworker would have to wait for the main thread to become "free" before it can do anything. Since the sheetworker always has to wait on the main thread to anything but logic (e.g. get an attribute, set an attribute), you can guarantee that things will happen in the correct order. So thinking about this, I actually do think it would work the way that you're suggesting in the last part of your post. I think the problem is that if we take my example above and insert an expensive operation between 3 and 4, the sheet worker would be sitting idly and the timeout would fire when the main thread resumes. I think I can switch it to a method where when it thinks the timeout should fire, it instead gives the main thread a chance to complete all current operations it wants to call, and then it should be fine. So I will try that instead. Hope that helps clear it up.
1489183342

Edited 1489183354
Riley D.
Roll20 Team
I pushed a really quick and dirty attempt at my idea for fixing this to Dev, but I have no idea if it worked or not since the test case I have doesn't have the issue. Give it a try and let me know. In addition if the problem is still there, I'd be curious to know if the same problem is happening in the browser client or not, too. Because at this point the API and the browser should work the same, so if there is a flaw in one I'd expect it to be in the other. Unless the API client is just that much slower or something.
1489560721
Lucian
Pro
API Scripter
Hi Riley, Sorry for the delay, a little tied up with other stuff in the past few days. Just tried to go and test this, but unfortunately: <a href="https://app.roll20.net/forum/post/4772789/error-52" rel="nofollow">https://app.roll20.net/forum/post/4772789/error-52</a>... :-( Hopefully will be sorted out later today and then I'll go back and have another look... Cheers, Lucian
1489564359
Lucian
Pro
API Scripter
Ok, I managed to work around that by going direct to the API scripts page for the campaign, and the answer is..... &lt;DRUM ROLL&gt; ... it looks fixed! Obviously, with this being a rather non-deterministic, load-dependent problem, it's hard to be 100% sure, but I've run several different heavy commands on a large test campaign, and I can't make the problem recur in any form now. Many thanks for persevering through this with me - particularly in the light of your recent change in focus. One thing that all of this has brought to light for me is that we have some work to do on our end. The sheetworkers are written at the moment to deal with users making changes field by field. When we create/modify lots of attributes at once via the API, (creating a bunch of spells for a character, for example) this generates a huge amount of redundant processing. I believe we can avoid this by having a special "API data" attribute which allows the sheetworkers to make changes in a single pass rather than "attribute by attribute". This should speed things up, make the behaviour a lot more deterministic, and reduce the load on the server. I'll be looking at that soon. Thanks again for all your help!
Awesome! Glad to hear it. Well I will probably wait until early next week to push this out to Main since I am going out of town for a few days starting tomorrow and I want to be around to babysit it due to the NodeJS upgrade. Glad that we got it figured out! /highfive