Slow for-Loop in 3dsmax 2017

[mail]

Just a headsup for members of this forum that you might want to check the performance of any scripts that make heavy use of loops before upgrading to 2017.

It's been raised on the main forum and on cgtalk that the for-loop is 2+ times slower than in 2016. ( in retrospect I should have created the original thread on this forum)

Threads here :

http://forums.cgsociety.org/showthread.php?f=98&t=1359224

http://forums.autodesk.com/t5/3ds-max-3ds-max-design-general/3ds-max-2017-w-sp1-almost-useable-but-n...

[kevinvandecar]

Hi Patrick,

I also saw similar results, but given that the loop is not doing anything, it's hard to say it is significantly slower. For example, on my machine the 1 million loop takes .489 second on 2017, and .260 second on 2016, so neither one is taking a significant amount of time for the looping itself.

But to see if there was a reason, I discussed this with our engineering team, and turns out this is intentional and was evaluated to be a benign performance issue, but added stability and improved error handling. The difference in this case, is due to new overhead per loop iteration that was added in 2017. In the internal implementation, what used to be a pair of inlined push/pop methods is now handled by an object being created and destroyed. Yes, there is a small amount of overhead due to this, but based on real tests the hit is insignificant compared to any operations that might be run inside the loop.

The reported results seem “big” because almost nothing else is going on, but as soon as you start doing actual work within the loop, this additional overhead should become insignificant.

For 3ds Max 2017, almost all the internal implementation of mxs push/pop code was replaced with class instances that do these operations in their ctor/dtors. This ensures that all code is doing things correctly, and that the code is exception safe. This leads us to increased stability and error handling.

Please let us know if you feel there are other performance problems not related to the actual looping code. Again, when testing these areas, we did not see any significant impact for real-world operations (ie. for looping code, the operations being performed are carried out in basically the same amount of time when adding the slight overhead of the 2017 changes).

hope it helps,

kevin

[mail]

Hi Kevin,

Thanks for looking in to this for me. I think I owe you guys an apology for jumping the gun on this one. You are quite right that the performance hit is insignificant in real world applications. The script I was working on showed a serious drop in performance when run in max 2017 and initial testing flagged this up as a possible cause. With further analysis I discovered some horrific code that I had failed to clean up ; I was using a try()catch() within a ui loop to prevent exceptions on an execute() that wasn't adequately formed and was triggering exceptions on each loop, thus triggering the catch()!

As this bit of code simply shouldn't exist you can ignore this issue, but I did notice that 2017 took far longer to handle this try(execute("garbage code"))catch() code. 

Please pass on my thanks to the engineering team for responding with such a detailed response, and for the optimisation! 

Cheers

p.

[Swordslayer]

Yet the slight overhead doesn't translate to 'basically the same amount of time' but for a 1.2 to 1.5x greater execution time in general case (with well-written real-world code and times in seconds, not milliseconds) and even more than 3 times slower execution when there are variables changed from a different scope (still well-written code with no issues) - see this post and others in the thread.

Don't get me wrong, I appreciate the benefits, it's just that I now have to go back and test my old code, try and identify the scripts that are two-odd times slower and try to do something about it. And I really don't want to use and abuse a while loop in place of a for loop just 'cause it might be twice as fast now...

Btw. it would be nice if we could continue the discussion in the cgsociety thread instead as autodesk forums are kinda PITA, I don't mind them being slow but in 5 in 7 browsers I tried (vivaldi, safari, firefox on mac, waterfox on windows - old firefox and edge were working ok), the rich text formatting tools (as well as the html tab) didn't work and I couldn't view post listing, only thread contents - anectodal evidence, but I don't think I'm the only one experiencing this... and people are commenting there and no one comments here.

[Swordslayer]

As an example, here's the difference when working with a 1M mesh in 2016 and 2017:

(
	local obj = convertToPoly (Plane lengthSegs:1000 widthSegs:1000)
	st = timeStamp(); sh = heapFree

	local selectedVerts = polyop.getVertSelection obj

	for i = 1 to 100 do
		for j in selectedVerts do
		(
			/* let's assume we do some stuff here */
		)

	format "time:% ram:%\n" (timeStamp() - st) (sh - heapFree)
)

It's fairly normal production-ready code and although the bitarray contents might change, for example discarding verts that have already been processed, this bitarray is already empty (it's .numberset is zero, it's count is and always will be 1M). Bitarrays are great for that as you can easily subtract from them so I'm not quite ready to give up using them and start converting any bigger returned bitarray to array. Now for the timing results:

Max 2016: time:696 ram:248L
Max 2017: time:8734 ram:264L

That's almost nine seconds in 2017 vs. less than a second in 2016 - not what I would call 'basically the same amount of time' (in this particular instance, it's 12.5 as much time to save you the calculations).

[spacefrog_]

The bitarray issue/slowdown is indeed a serious thing , and i can't imagine that this is only because of internal code safety improvements with Max2017

There has to be some deeper issue been introduced with the changes which certainly show with the slowdown factor of >10x

[mail]

Just to chip in again, it might also be worth looking at the performance of failed try()catch() statements. 

I appreciate that these are perhaps NEVER expected to be used in final code, but in some circumstances it may be necessary for expediancy... maybe not! 

There is definately a huge performance drop between 2016 and 2017 where I've had try()catch() statements excepting and calling the catch() statement. 

I am now going through my code and removing all try()catch()'s as I appreciate they should be removed, but it's a shame that they now introduce such a performance hit. 

[kevinvandecar · Accepted solution]

Hi Guys, a little more information here... Seems a lot of changes came to 2017 MAXScript. Another area that was touched was the exception handling mechanism. Now, the complete mxs callstack is captured at the time the exception occurs. Previously, each level of the mxs callstack was captured as we backed out through the call stack.  Because we are capturing the complete call stack, in some cases we capture more levels than previously. If those levels contained lots of local variables, or the values in the local variables contain lots of values, this capture can take longer that it took in 2016.  The case where this has the largest impact is with code looking like:  try(…expression…)catch(). Previously, the callstack capture would end at this point, but now the stack levels above this are captured. This is particularly bad because the try/catch is just for eating the exception and we don’t care about the callstack capture. But, we don’t know whether in the catch the user is calling getCurrentExceptionStackTrace(), so we have to capture the stack.

We introduced the ‘MXSCallstackCaptureEnabled’ context to help with this type of case.  When in this context, the call stack is not captured if an exception is thrown. An example usage would be:

try with MXSCallstackCaptureEnabled off (setproperty obj #prop val)

catch() -- just eat any errors

I would suggest to review this section in docs about the MXSCallstackCaptureEnabled:

http://help.autodesk.com/view/3DSMAX/2017/ENU/?guid=__files_GUID_F2315A09_440B_4EAE_8873_F27265CDB66...

Also note there is a "what's new in MAXScript 2017" here:

http://help.autodesk.com/view/3DSMAX/2017/ENU/?guid=__files_GUID_88FE9C40_FDC0_4858_9675_493A9546DF0...

I would not remove all try/catch, because they are meant to be used for error handling, but I would be careful about the complexity of the code contained within the block... maybe break it down into smaller try/catch blocks could help. And of course use the new MXSCallstackCaptureEnabled option when appropriate.

hope it helps,

kevin

[mail]

Thanks for the detailed reponse again Kevin; much appreciated! I think that clears up my understanding of the issue as well as providing a decent solution.

Thanks!

p.

[Swordslayer]

I would not mark that as a solution to the thread, try-catch has nothing to do with with the slowness of iterating arrays/bitarrays, everything's described in the helpfile (well, the 'what's new' section is the first thing I read as soon as new helpfile is available for download, I am well aware of this - and as I haven't used a try-catch in the last seven years anywhere where it would matter, I won't be using it anyway). I don't need a solution for the slow bitarray iterating now, just say 'we are now aware of it and a fix is in the works (or scheduled, whatever'.

[mail]

Sorry, you are quite right. I shouldn't have clicked the solved button! (mods : can you unsolve this issue?)

Further to that, I've thought a bit more about this implementation, and it seems a bit odd that the new stack trace feature was an opt-out rather than opt-in feature... surely this will cause havoc with legacy scripts and require much rewriting to negate the slowdown.

Surely, users only need the new stack-trace feature when developing, so the new feature should only be necessary for new scripts during development; eg a single toggle at the beginning and end of the script to enable then disable the more detailed stack-trace would be a better approach as the stack-trace could be disabled entirely when the script is released in the wild giving the code a little speed boost in the process.

The way it has been implemented has the potential to render hundreds of scripts unusably slow.

[Swordslayer]

Although I kind of agree that within try-catch operations the MXSCallstackCaptureEnabled context could be implicitly off, the 'hundreds of scripts' is a hyperbole, it would have to be a really badly written script to suffer from this. Apart from disableRefMsgs() / enableRefMsgs() pair which needs to be executed no matter what happens (and that usually only needs to be done once during the runtime of the script) and for destroying global rollout dialogs which is now kind of an idiom for its convenience (and I don't see anyone cdestroying more than a few rollouts either), where else would you use it in production code?

[mail]

Fair enough. I've been guilty of using try/catch in lieu of correctly handling badly cast variables in the past, so I am sure others have too.

I think my point is that given the change can hit performance, and isn't strictly required for a correctly functioning script (unless I am missing something), then the default state should be as though disableRefMsgs is active. Only when a script is being developed would you want to enableRefMsgs to help debug your code.

But yes, this is a minor issue given the performance issue only hits code that is disgracefully poor in the first place 🙂

[kevinvandecar]

Hi Vojtech,

I think the solution for this thread is OK here because it started mainly as "slow for-loop". We also answered the "exception" issue as a bonus. 🙂

After wading through the discussion at cgsociety, I think the last thing to deal with is the "iterating arrays/bitarrays" issue that you also mentioned here. I ran that issue by our developers, and one guy did some testing with automation test scripts he had using much array and bitarray using a code performance analysis tool (VTune). In the end, yes, it was slower, but his impression was that it had to do with all the other changes and not anything directly with array/bitarray.

So, if you have any examples that demonstrate what you feel is a significant performance issue, feel free to pass it over and I will give to engineering for more review.

hope it helps,

thanks,

kevin

[Swordslayer]

Hi, see the previous page, this post. Note that all it gets to get from half a second in max 2016 (time that you wouldn't notice) to close to nine seconds (time that would leave you counting to ten). Is to use any polyop method that returns a bitarray on big enough mesh (and let's be honest, big meshes are usually the reason I'm using maxscript to begin with) - iterating the bitarray, the for j in selectedVerts do for loop, is where the slowdown happens. For another real-world code, see this post #48 in the original thread. The GetElements function there uses bitarrays, if you rewrite it to use arrays instead, the speed difference factor between versions is the usual 1.5, with bitarrays it is 4 - and the performance gap gets wider as the bitarray count increases - the time increase in 2016 is logarithmic and in max 2017 it's linear.

I don't really care what causes it, this shouldn't happen.

[mail]

To return to the try()catch() performance degredation, here's an example of where it's required, and where the use of "with MXSCallstackCaptureEnabled off" is inadequate. 

Where I'm iterating over all the scene objects and modifiers and getting or setting a parameter, I can usually use "hasproperty", but for some classes this doesn't work; for example the cloth modifier.

Create a box and add a cloth modifier.

m=$.modifiers[1]

hasproperty m "bias"

getproperty m "bias"

As you can see, even though hasproperty returns true, when I attempt to get the property, I get an exception. 

My script aims to work without hard-coding every class property, so it is essential that I can use maxscript to a) list the class properties, then b) check that an object has that property before c) getting or setting that property. 

As the behaviour of some classes(maybe only the cloth modifier, but maybe others) does not allow me to reliably check for the existance of a property, I have to set get/setproperty within a try/catch. 

So... this was all well and good in 3dsmax versions up to 2017 as try/catch doesn't degrade performance too much. In 2017 I can't use try/catch anymore, so I either have to hard-code a test for invalid classes (something I can't do till I actually stumble across this bug), or I use the new MXSCallstackCaptureEnabled method to bring the try/catch performance closer to what it was in 2016 and before.

I would do this, but this now introduces code that isn't supported in versions prior to 2017 so I am now forced to add another test to return the max version and to run a different copy of the same chunk of code depending on the max version. This does not seem like a good place to end up!

If the new stack-trace method was disabled by default, none of this would be an issue, and where I was developing in a known 2017 environment, I would be able to enable the enhanced stack-trace as required. Assuming I am not missing another far easier approach to circumvent these issues, can I request that you pass this back to your developers and ask them to consider reverting to the original stack-trace behaviour with the new behaviour only active when explicitly called. 

[mail]

Here's an example. If I am iterating over all scene classes and getting properties, I need to check the property exists before calling it. This works most of the time, but there are some exceptions in which case the only way I can do this without a hard-coded list of exceptions is to use try/catch.
As my script doesn't know what classes and their properties to expect, I need to be able to programatically discard invalid parameters on the fly.

Create a box, add a cloth modifier

m=$.modifiers[1]
hasproperty m "bias"
getproperty m "bias"

[mail]

Actually, it looks like I can't even run different code for older versions of max with this workaround :

if ((maxVersion())[1] / 1000) > 18 then (with MXSCallstackCaptureEnabled off try(format "test")catch())

This fails in max2016.

[Swordslayer]

Ad 1) that's what isProperty is for, hasProperty gives you properties of the class no matter if they are currently accessible or not (which is most useful when you are calling class constructor), isProperty tries to access the property silently and tells you if it succeeded (which also means it will work with inherited properties). As a side note, I hope you don't use $ in the script itself.

Ad 2) bummer, the parser doesn't know that context which means there would need to be a different version of the script for 2017+ if you insisted on using try-catch and this context.

Anyway, this doesn't belong in this thread, make a new thread for this.