Sign in to follow this  

[PATCH] POD members in object memory

This topic is 1495 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Inspecting some of the bytecode output from script class constructors one thing I noticed was that application POD classes were being allocated on the heap in their own little chunks of memory. This was mainly an issue for small things like 2d/3d vectors, rectangles, etc. The worst offender of it all being our 'Color' class - a shortcut for representing colors as 32-bit rgba integers which was being allocated into its own heap memory and adding a 64-bit pointer to the class.

 

The attached patch changes the behaviour of POD application class typed members to be stored directly in the object's allocated memory block instead. It passes angelscript's test suite, and we've been using it in our project for a few weeks now.

 

Feel free to use/alter if you're interested in using it.

 

 

 

 

Two small miscellaneous things that I'm going to shoehorn into this thread:

 

The test suite needs C++11 to compile, so the makefile needs to tell gcc to use it:

Index: sdk/tests/test_feature/projects/gnuc/makefile
===================================================================
--- sdk/tests/test_feature/projects/gnuc/makefile	(revision 1759)
+++ sdk/tests/test_feature/projects/gnuc/makefile	(working copy)
@@ -1,7 +1,7 @@
 # Test Framework GNUC makefile
 
 CXX = g++
-CXXFLAGS += -ggdb -I../../../../angelscript/include -Wno-missing-field-initializers
+CXXFLAGS += -std=c++11 -ggdb -I../../../../angelscript/include -Wno-missing-field-initializers
 SRCDIR = ../../source
 OBJDIR = obj

Also, there's a problem with the context used in ScriptObjectFactory: if script engine A calls an application function that wants to create a script object in script engine B, ScriptObjectFactory will nest into the context from engine A, creating a crash. This occurred in some of our cross-engine communication functions. We added a check to not use nesting if the active context is from a different engine, but there should probably be a way for the application to provide asIScriptEngine::CreateScriptObject() with the context to use for the call since creating contexts is expensive.

Index: sdk/angelscript/source/as_scriptobject.cpp
===================================================================
--- sdk/angelscript/source/as_scriptobject.cpp	(revision 1759)
+++ sdk/angelscript/source/as_scriptobject.cpp	(working copy)
@@ -53,12 +53,17 @@
 	ctx = asGetActiveContext();
 	if( ctx )
 	{
-		r = ctx->PushState();
+		if( ctx->GetEngine() == engine )
+		{
+			r = ctx->PushState();
 
-		// It may not always be possible to reuse the current context, 
-		// in which case we'll have to create a new one any way.
-		if( r == asSUCCESS )
-			isNested = true;
+			// It may not always be possible to reuse the current context, 
+			// in which case we'll have to create a new one any way.
+			if( r == asSUCCESS )
+				isNested = true;
+			else
+				ctx = 0;
+		}
 		else
 			ctx = 0;
 	}

Share this post


Link to post
Share on other sites

I haven't even completely integrated your previous patch yet and here you're providing another one. :)

 

Changing the code to allocate the value types inline has been on my to-do list for quite some time. Thanks a lot for saving this time for me. 

 

I'll probably not include this for 2.28.0 which is near completion, but I'll definitely look into adding it for the next release after that.

Share this post


Link to post
Share on other sites

I've included this patch in revision 1799.

 

You seem to have a really good grasp of the internal workings of AngelScript. :)

Share this post


Link to post
Share on other sites
Sign in to follow this