DynamoDS / DynamoWishlist

This is a repository for all wishlist items for Dynamo Core
https://github.com/orgs/DynamoDS/projects/3
Apache License 2.0
15 stars 3 forks source link

Enable FullFrames on IronPython Engine #152

Open gtalarico opened 7 years ago

gtalarico commented 7 years ago

I am working on a interactive debugger console. I have it working with the IronPythonShell but unfortunately is not working in Dynamo because because the engine does not have stack frames enabled.

It's a simple change that would allow stack inspection and the development of additional debugging tools and features; I think the Dynamo community could really benefit from.

I have never developed in C# or built a project like Dynamo, but I am happy to try. Could you let me know if something like this could get merged, and if the code below makes sense?

https://github.com/DynamoDS/Dynamo/blob/dcb404259570d0252e5e8500cfd64bf46c5171ad/src/Libraries/DSIronPython/IronPythonEvaluator.cs#L49

// Add: Create Options Dict (Need to ScriptingRuntimeHelpers
var options = new Dictionary();
options["FullFrames"] = ScriptingRuntimeHelpers.True;

// Line 49 on IronPythonEvaluator.cs
// ScriptSource script = Python.CreateEngine().CreateScriptSourceFromString(code);
// Change to:
ScriptSource script = Python.CreateEngine(options).CreateScriptSourceFromString(code);

RevitPythonShell enables by passing the argument in the same way you would in the Command line, but It's just starting the python engine instead of passing code directly to it:

this.Run(new string[] { "-X:FullFrames" });

https://github.com/architecture-building-systems/revitpythonshell/blob/c8753ba227e93cd4266a73847e1da55fc59466e2/PythonConsoleControl/PythonConsoleHost.cs#L121

mjkkirschner commented 7 years ago

Neat! Is there some information on the performance impact of this change?

gtalarico commented 7 years ago

Some quick timeit tests show a small impact, possibly negligible, but I that's not a proper sample size. I guess we could do proper benchmarking if the team is interested in pursuing.

If it turns out it has a significant impact, we could always add it as a toggleable option. PS: Added a sample of the debugger in action at the bottom

Python 3 - Just for fun.


C:\test\docs (dev/mergebitbucket)
(revitpythonwrapper) λ python -m timeit "'-'.join([str(n) for n in range(100)])"
10000 loops, best of 3: 22.3 usec per loop

C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ python -m timeit "'-'.join([str(n) for n in range(100)])" 10000 loops, best of 3: 21.9 usec per loop

C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ python -m timeit "'-'.join([str(n) for n in range(100)])" 10000 loops, best of 3: 22 usec per loop

>>> IronPython 2.7.7 - Normal

C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ipy -m timeit "'-'.join([str(n) for n in range(100)])"

:1: RuntimeWarning: IronPython has no support for disabling the GC 10000 loops, best of 3: 21.2 usec per loop C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ipy -m timeit "'-'.join([str(n) for n in range(100)])" :1: RuntimeWarning: IronPython has no support for disabling the GC 10000 loops, best of 3: 21.2 usec per loop C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ipy -m timeit "'-'.join([str(n) for n in range(100)])" :1: RuntimeWarning: IronPython has no support for disabling the GC 10000 loops, best of 3: 21.2 usec per loop C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ipy -m timeit "'-'.join([str(n) for n in range(100)])" :1: RuntimeWarning: IronPython has no support for disabling the GC 10000 loops, best of 3: 21.2 usec per loop ``` >>> IPY + FULL FRAMES ``` C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ipy -X:FullFrames -m timeit "'-'.join([str(n) for n in range(100)])" C:\Program Files (x86)\IronPython-2.7.7\Lib\timeit.py:1: RuntimeWarning: IronPython has no support for disabling the GC #! /usr/bin/env python 10000 loops, best of 3: 21.2 usec per loop C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ipy -X:FullFrames -m timeit "'-'.join([str(n) for n in range(100)])" C:\Program Files (x86)\IronPython-2.7.7\Lib\timeit.py:1: RuntimeWarning: IronPython has no support for disabling the GC #! /usr/bin/env python 10000 loops, best of 3: 21.6 usec per loop C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ipy -X:FullFrames -m timeit "'-'.join([str(n) for n in range(100)])" C:\Program Files (x86)\IronPython-2.7.7\Lib\timeit.py:1: RuntimeWarning: IronPython has no support for disabling the GC #! /usr/bin/env python 10000 loops, best of 3: 21.4 usec per loop C:\test\docs (dev/mergebitbucket) (revitpythonwrapper) λ ``` ![console](https://user-images.githubusercontent.com/9513968/27263465-7922b670-5437-11e7-825a-4536fe1ef11f.gif)
Racel commented 7 years ago

After talking with @mjkkirschner, performance looks ok, and we would like to move forward with this. Can you submit a PR?

gtalarico commented 7 years ago

@Racel @mjkkirschner I tried building Dynamo to test this out but was not able to. My VS game is pretty weak :) Given this is a 2 line mod, could your team try to get it in there for me?

QilongTang commented 7 years ago

@gtalarico I created the feature branch https://github.com/DynamoDS/Dynamo/tree/EnableFullFrames for you to build and testing. It contains the code you want to add when creating the engine, not completely same with what you put in description due the .Net API changes. Could you get the branch and build Dynamo to test so you can let us know if that is doing what you proposed?

FYI: @Racel @mjkkirschner