Skip to content

Commit

Permalink
Improve naive reference counting performance
Browse files Browse the repository at this point in the history
Profiling the attached mx_utils_SHA256 test shows that considerable
amount of execution time is spent in acquireObject/abandonObject.

This commit converts referencedObjects to an intrusive linked list.
Benefits: O(1) insertion and deletion, and avoiding memory
allocations.

A lot of time is still being spent on locking.
  • Loading branch information
aajanki committed Sep 19, 2012
1 parent b10f257 commit 2a0d8cb
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 25 deletions.
49 changes: 37 additions & 12 deletions README.tests
Expand Up @@ -3,23 +3,48 @@ http://opensource.adobe.com/wiki/display/flexsdk/Download+Flex+4

Flex 4 SDK is assumed to be installed in $FLEX_ROOT

To compile tests inside the tests/ directory use the following command:
To compile and run the test suite run:

./tests

Run "./tests --help" inside tests/ to see scripts usage.

Compiling tests manually inside the tests/ directory:
$FLEX_ROOT/bin/mxmlc -static-link-runtime-shared-libraries -compiler.omit-trace-statements=false test.mxml -output test.swf

Creating new testcases
----------------------

Use template.mxml as a template for new testcases.

We've provided a simple Tests class which should be used where
possible to test general behaviour of AS classes. See Tests.as for its
usage.

Please use a naming scheme as follow:

For toplevel classes: tests/CLASSNAME_test.mxml (CLASSNAME is the
exact name of the class/function)

For other classes: tests/NAMESPACE_CLASSNAME_test.mxml (NAMESPACE is
for example "net" for "flash.net" classes/functions, e.g.:
tests/net_NetStream_test.mxml)

Please group all tests for one class in one file.

Subdirectories
--------------

NOTE: Use the ./tests script in tests/ to run the tests in an automated way.
Run "./tests --help" inside tests/ to see its usage.
tests/other:

NOTE: Use template.mxml as a template for new testcases.
Tests that aren't suitable for testing with the Tests class should be
put in here. (e.g.: testing video playing, StageScaleMode testing
etc...). The naming schemes still apply to these testcases though.

NOTE: We've provided a simple Tests class which should be used where possible to test general behaviour of AS classes. See Tests.as for its usage.
tests/unimplemented:

NOTE: Please use a naming scheme as follow:
For toplevel classes: tests/CLASSNAME_test.mxml (CLASSNAME is the exact name of the class/function)
For other classes: tests/NAMESPACE_CLASSNAME_test.mxml (NAMESPACE is for example "net" for "flash.net" classes/functions, e.g.: tests/net_NetStream_test.mxml)
Please group all tests for one class in one file.
Tests for unimplemented features.

NOTE: Tests that aren't suitable for testing with the Tests class should be put in tests/other. (e.g.: testing video playing, StageScaleMode testing etc...)
The naming schemes still apply to these testcases though.
tests/performance:

NOTE: Tests for unimplemented features should be placed in tests/unimplemented.
Tests aimed at measuring runtime performance.
3 changes: 2 additions & 1 deletion src/asobject.h
Expand Up @@ -26,6 +26,7 @@
#include "threading.h"
#include "memory_support.h"
#include <map>
#include <boost/intrusive/list.hpp>

#define ASFUNCTION(name) \
static ASObject* name(ASObject* , ASObject* const* args, const unsigned int argslen)
Expand Down Expand Up @@ -240,7 +241,7 @@ enum METHOD_TYPE { NORMAL_METHOD=0, SETTER_METHOD=1, GETTER_METHOD=2 };
//for toPrimitive
enum TP_HINT { NO_HINT, NUMBER_HINT, STRING_HINT };

class ASObject: public memory_reporter
class ASObject: public memory_reporter, public boost::intrusive::list_base_hook<>
{
friend class ABCVm;
friend class ABCContext;
Expand Down
27 changes: 17 additions & 10 deletions src/scripting/toplevel/toplevel.cpp
Expand Up @@ -695,14 +695,14 @@ const Type* Type::getTypeFromMultiname(const multiname* mn, const ABCContext* co
}

Class_base::Class_base(const QName& name, MemoryAccount* m):ASObject(Class_object::getClass()),protected_ns("",NAMESPACE),constructor(NULL),
referencedObjects(std::less<ASObject*>(), reporter_allocator<ASObject*>(m)),borrowedVariables(m),
borrowedVariables(m),
context(NULL),class_name(name),memoryAccount(m),length(1),class_index(-1),isFinal(false),isSealed(false),use_protected(false)
{
type=T_CLASS;
}

Class_base::Class_base(const Class_object*):ASObject((MemoryAccount*)NULL),protected_ns("",NAMESPACE),constructor(NULL),
referencedObjects(std::less<ASObject*>(), reporter_allocator<ASObject*>(NULL)),borrowedVariables(NULL),
borrowedVariables(NULL),
context(NULL),class_name("Class",""),memoryAccount(NULL),length(1),class_index(-1),isFinal(false),isSealed(false),use_protected(false)
{
type=T_CLASS;
Expand Down Expand Up @@ -878,26 +878,33 @@ void Class_base::handleConstruction(ASObject* target, ASObject* const* args, uns
void Class_base::acquireObject(ASObject* ob)
{
Locker l(referencedObjectsMutex);
bool ret=referencedObjects.insert(ob).second;
assert_and_throw(ret);
assert_and_throw(!ob->is_linked());
referencedObjects.push_back(*ob);
}

void Class_base::abandonObject(ASObject* ob)
{
Locker l(referencedObjectsMutex);
set<ASObject>::size_type ret=referencedObjects.erase(ob);
if(ret!=1)
assert_and_throw(ob->is_linked());
#ifdef EXPENSIVE_DEBUG
//Check that the object is really referenced by this class
int count=0;
for (auto it=referencedObjects.cbegin(); it!=referencedObjects.cend(); ++it)
{
LOG(LOG_ERROR,_("Failure in reference counting in ") << class_name);
if ((&*it) == ob)
count++;
}
assert_and_throw(count==1);
#endif

referencedObjects.erase(referencedObjects.iterator_to(*ob));
}

void Class_base::finalizeObjects() const
void Class_base::finalizeObjects()
{
while(!referencedObjects.empty())
{
set<ASObject*>::iterator it=referencedObjects.begin();
ASObject* tmp=(*it);
ASObject *tmp=&referencedObjects.front();
tmp->incRef();
//Finalizing the object does also release the classdef and call abandonObject
tmp->finalize();
Expand Down
5 changes: 3 additions & 2 deletions src/scripting/toplevel/toplevel.h
Expand Up @@ -32,6 +32,7 @@
//#include "scripting/toplevel/XML.h"
#include "memory_support.h"
#include <libxml++/parsers/domparser.h>
#include <boost/intrusive/list.hpp>

namespace lightspark
{
Expand Down Expand Up @@ -152,8 +153,8 @@ template<class T> friend class Template;
void describeMetadata(xmlpp::Element* node, const traits_info& trait) const;
//Naive garbage collection until reference cycles are detected
Mutex referencedObjectsMutex;
std::set<ASObject*, std::less<ASObject*>, reporter_allocator<ASObject*>> referencedObjects;
void finalizeObjects() const;
boost::intrusive::list<ASObject, boost::intrusive::constant_time_size<false> > referencedObjects;
void finalizeObjects();
protected:
void copyBorrowedTraitsFromSuper();
ASFUNCTION(_toString);
Expand Down
30 changes: 30 additions & 0 deletions tests/performance/mx_utils_SHA256_test.mxml
@@ -0,0 +1,30 @@
<?xml version="1.0"?>
<mx:Application name="lightspark_utils_SHA256_test"
xmlns:mx="http://www.adobe.com/2006/mxml"
layout="absolute"
applicationComplete="appComplete();"
backgroundColor="white">

<mx:Script>
<![CDATA[
import flash.system.fscommand;
import mx.utils.SHA256;
import flash.utils.ByteArray;
private function appComplete():void
{
var ba:ByteArray = new ByteArray();
for (var i:int=0; i<10000; i++) {
ba.writeUTFBytes("1234567890");
}
SHA256.computeDigest(ba);
fscommand("quit");
}
]]>
</mx:Script>

<mx:UIComponent id="visual" />

</mx:Application>

1 comment on commit 2a0d8cb

@alexp-sssup
Copy link
Member

Choose a reason for hiding this comment

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

Indeed. I have currently an half-baked solution that makes it not necessary to keep track of all the objects manually by integrating with a slightly custom version of jemalloc. I'll try to commit both the modified jemalloc and the relevant code in the next few days

Please sign in to comment.