Skip to content
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

Use configurable MaximumRecursion #368

Merged
merged 9 commits into from Apr 19, 2020
Merged

Use configurable MaximumRecursion #368

merged 9 commits into from Apr 19, 2020

Conversation

tznind
Copy link
Contributor

@tznind tznind commented Apr 14, 2020

This PR addresses the issue where you need to give a large object with many child methods as a global variable but want to restrict the amount of up front reflection that happens (for performance).

#366

I have used System.DataTable in the test since it is a good complex class with self referencing properties.

Setting MaximumRecursion to 0 on an instance of Lua results in only a single global being created and no reflection exploration happening (see TestMaximumRecursion).

@@ -45,6 +45,7 @@
<HintPath>..\..\..\packages\NUnit.3.12.0\lib\net45\nunit.framework.dll</HintPath>
</Reference>
<Reference Include="System" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the System.Data to test this? Can't we use a POCO object? NLua tests will run on Linux, Mac including iOS and tvOS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I will fix this to use one of the test classes.

Copy link
Contributor Author

@tznind tznind Apr 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I changed to use the test class but Mac is still failing. Do I need import ('mscorlib','System') in DoString? I'm not sure what I am doing wrong, any help would be much appreciated. I can't see the build log from Mac so am a bit blind here.

if(maxRecursion == 0)
Assert.AreEqual(1,lua.Globals.Count()); //register only the root reference
else
Assert.Greater(lua.Globals.Count(),1); //many globals registered (all sub properties)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NUnitLite doesn't have Greater you can just do True (lua.Globals.Count() > 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for helping me with this PR. Looks like there are some failing tests with PlatformNotSupportedException in NLua.CodeGeneration constructor now...

@tznind
Copy link
Contributor Author

tznind commented Apr 16, 2020

Ah looks like those PlatformNotSupported failing tests are also failing in your master branch: https://github.com/NLua/NLua/runs/463903787

@viniciusjarina viniciusjarina merged commit 7ff6e7c into NLua:master Apr 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants