🔒 Locked

Memory allocation headache.

Started by HappyCoder Feb 27, 2008 at 8:53 PM 10 replies 1.8k views

Original Post

Feb 27, 2008 at 8:53 PM

alright, so I am working on a calculator. I was originally had a system that would pass values (complex numbers, lists, matrices) using the stack like this. This is the function I was using


void GetValue(CNumber* result)
{
   // logic code here
   // result is usually a class allocated on the stack by the caller
   // would look something like this
   // CNumber number = CComplexNumber(0.0, 1.0);
   // GetValue(&number);
   *result = (return value);
}

This would work but I polymorphism wasn't working with my lists and matrices so I put ALL the code for complex numbers, lists, and matrices into one source file. This would work but my code was a pain to manage and it was difficult to add now types (strings) to my calculator. So I decided to have get value work like this


CNumber* GetValue()
{
   // logic code here
   // involves creating an object with the 'new' operator
   // caller function will take returned value, use it, then delete it.
   return (return value);
}

This is how I have it set up right now. This works great. Except it is way slower now. I suspect it has something to do with having to create now objects and them deleting them. Can anybody suggest any way to improve upon this system?

swiftcoder

Feb 27, 2008 at 11:19 PM

There is something funky going on in your code, and I am not entirely sure why it compiles. You need to read up on the return statement - in particular, the statement return (return value) is completely meaningless.

Edit: Kudos to Hodgman, I think too literally at times...

Hodgman

Feb 27, 2008 at 11:22 PM

Quote:
Original post by swiftcoder
There is something funky going on in your code, and I am not entirely sure why it compiles. You need to read up on the return statement - in particular, the statement return (return value) is completely meaningless.

I'm pretty sure that "(return value)" is meant as psuedo-code ;)

Zakwayda

Feb 28, 2008 at 12:16 AM

Quote:
Original post by Hodgman
I'm pretty sure that "(return value)" is meant as psuedo-code ;)

Heh...I read it literally too :)

swiftcoder

Feb 28, 2008 at 7:53 AM

Quote:

Original post by HappyCoder

CNumber* GetValue(){   // logic code here   // involves creating an object with the 'new' operator   // caller function will take returned value, use it, then delete it.   return (return value);}

Beware transferring ownership in this way, by returning a pointer that must be deleted externally, as forgetting to delete the value outside of the function is very easy to do, or even deleting it twice. Instead you should be returning a std::auto_ptr, as it will ensure that the object is deleted unless you specifically request otherwise.

Antheus

Feb 28, 2008 at 8:22 AM

Quote:
Can anybody suggest any way to improve upon this system?

Perhaps, but you'll need to elaborate a bit more on what you want. A single method without any real context isn't helpful - both approaches can be used.

I'm not even remotely clear what this should do, and how it applies to matrices, vectors, and everything else.

Quote:
Except it is way slower now.

This would be a calculator, the thing that has a few buttons and an LCD screen?

How do you measure "slowness". You'd be doing thousands of calculations at most (let's think big here). On any computer from last 10 years the above should take milliseconds.

Also - how do you delete the allocated values? Do you even do that? If not, then the slowness comes from simply allocating more memory, eventually requiring data to be paged to disk, and eventually crashing.

HappyCoder

Feb 28, 2008 at 3:58 PM

It is a graphing calculator on the pocket pc. I got it working pretty well with the previous method. It ran very well. I could calculate 1000+ points in decent time for animated graphs. I made the change to my code to make it more flexible but now it is way slower.

I would say it takes about half a second to do now what was seemingly instant before. The only change I made was before I was using the stack to store values, now it am using the heap. The allocation/deallocation of the memory slowed it down quite a bit. Do you think there could be a way I could manage the memory myself with still maintaining classes with polymorphism?

Zahlman

Feb 28, 2008 at 5:26 PM

Could we see more of the code? How are these objects being created and destroyed?

HappyCoder

Feb 28, 2008 at 6:55 PM

Here is a pretty good example of what I am doing.

COperator.cpp

CNumber* COperator::GetValue(){	CNumber* result = NULL;	CNumber* left = m_Left->GetValue();	CNumber* right = m_Right->GetValue();	switch (m_dwOperator)	{	case OP_ADD:		result = left->Add(right);		break;	case OP_SUB:		result = left->Sub(right);		break;	case OP_MUL:		result = left->Mul(right);		break;	case OP_DIV:		result = left->Div(right);		break;	case OP_POW:		if (right->GetType() == NUM_COMPLEX)			result = left->cpow((CComplexNumber*)right);		else		{			delete left;			delete right;			throw new CError(ERR_INVALIDTYPE);		}		break;	}	delete left;	delete right;	return result;}

CComplexNumber.cpp

CNumber* CComplexNumber::Mul(CNumber* other){	switch (other->GetType())	{	case NUM_COMPLEX:		{			CComplexNumber* cmplx = (CComplexNumber*)other;			return new CComplexNumber(m_real * cmplx->m_real - m_imaginary * cmplx->m_imaginary, m_real * cmplx->m_imaginary + cmplx->m_real * m_imaginary);		}	case NUM_LIST:		{			CList* src = (CList*)other;			CList* list = new CList(src->GetSize());			for (int i = 0; i < list->GetSize(); i++)			{				list->SetElement(i, src->GetElement(i));				list->GetElement(i)->MulEq(this);			}			return list;		}	case NUM_MATRIX:		{			CMatrix* src = (CMatrix*)other;			CMatrix* mat = new CMatrix(src->GetRow(), src->GetCol());			for (int x = 0; x < mat->GetCol(); x++)			{				for (int y = 0; y < mat->GetRow(); y++)				{					mat->SetElement(y, x, src->GetElement(y, x));					mat->GetElement(y, x)->MulEq(this);				}			}			return mat;		}	}	throw new CError(ERR_INVALIDTYPE);}

Hopefully this is enough to get a grasp of what I am doing.

I have an idea of how I might be able to get this to work, but I am not sure if it is a good idea and I am unsure about doing it this way.

I pre-allocate some space to use for storing values as I calculate equations. This space is then filled with values and such. When I want to create a new number or list or whatever rather then saying 'new CList(10)' I write my own function like this 'CList::NewList(memoryManager, 10)'. The memory manager will allow me to work in more like a stack system that would be faster than using the heap and it would also allow me to still use polymorphism. The only thing I am unsure on how to do is set the correct values for the virtual method table for the objects I allocate.

Hacksaw2201

Feb 28, 2008 at 11:41 PM

I ran into a similar problem to yours when I was trying to pass a string back to the caller. Here is how I went about getting around the problem.

int SomeAPI(pointer* pnt){*pnt = class variable value or API valuereturn TRUE;}//allocation with new operatorin = new int;// or allocation at compile time (might be faster)int in;if ( SomeAPI(in) == TRUE){hey my stuff worked}delete in;

It was an odd work around at first but then I got to thinking about how valuable coding something this way would be. You always have the storage defined that the caller needs and the return value can be used as an error message of some sort within the caller. With this instance you can return something other than TRUE to state whether or not the function, method, or API worked appropriately thus making debugging easier.

Enigma

Feb 29, 2008 at 5:30 PM

This is exactly the kind of situation where I use boost::variant:

#include <algorithm>#include <iostream>#include <iterator>#include <list>#include <numeric>#include <boost/variant.hpp>typedef boost::variant< int, std::list< int > > value;struct operation_visitor;class operation{	public:		static operation multiply_operation(value const & lhs, value const & rhs);		value operator()() const;	private:		operation(value const & lhs, value const & rhs, operation_visitor const & visitor);		value lhs_;		value rhs_;		operation_visitor const * visitor_;};struct operation_visitor	:	public boost::static_visitor< value >{	virtual value operator()(int lhs, int rhs) const = 0;	virtual value operator()(int lhs, std::list< int > const & rhs) const = 0;	virtual value operator()(std::list< int > const & lhs, int rhs) const = 0;	virtual value operator()(std::list< int > const & lhs, std::list< int > const & rhs) const = 0;};struct multiply_visitor	:	public operation_visitor{	virtual value operator()(int lhs, int rhs) const	{		return lhs * rhs;	}	virtual value operator()(int lhs, std::list< int > const & rhs) const	{		return lhs * std::accumulate(rhs.begin(), rhs.end(), 1, std::multiplies< int >());	}	virtual value operator()(std::list< int > const & lhs, int rhs) const	{		std::list< int > result;		std::transform(lhs.begin(), lhs.end(), std::back_inserter(result), std::bind2nd(std::multiplies< int >(), rhs));		return result;	}	virtual value operator()(std::list< int > const & lhs, std::list< int > const & rhs) const	{		std::list< int > result;		std::transform(lhs.begin(), lhs.end(), rhs.begin(), std::back_inserter(result), std::multiplies< int >());		return result;	}};multiply_visitor const static_multiply_visitor;operation operation::multiply_operation(value const & lhs, value const & rhs){	return operation(lhs, rhs, static_multiply_visitor);}value operation::operator()() const{	return boost::apply_visitor(*visitor_, lhs_, rhs_);}operation::operation(value const & lhs, value const & rhs, operation_visitor const & visitor)	:	lhs_(lhs),	rhs_(rhs),	visitor_(&visitor){}std::list< int > make_list_value(){	std::list< int > list;	list.push_back(7);	list.push_back(9);	return list;}struct print_value_visitor	:	public boost::static_visitor<>{	void operator()(int value) const	{		std::cout << value << '\n';	}	void operator()(std::list< int > const & value) const	{		std::copy(value.begin(), value.end(), std::ostream_iterator< int >(std::cout, ", "));		std::cout << '\n';	}};int main(){	value int_value = 3;	value list_value = make_list_value();	operation multiply_ints = operation::multiply_operation(int_value, int_value);	operation multiply_int_by_list = operation::multiply_operation(int_value, list_value);	operation multiply_list_by_int = operation::multiply_operation(list_value, int_value);	operation multiply_list_by_list = operation::multiply_operation(list_value, list_value);	boost::apply_visitor(print_value_visitor(), multiply_ints());	boost::apply_visitor(print_value_visitor(), multiply_int_by_list());	boost::apply_visitor(print_value_visitor(), multiply_list_by_int());	boost::apply_visitor(print_value_visitor(), multiply_list_by_list());}

Σnigma

HappyCoder

Mar 1, 2008 at 3:25 PM

Well, I got it working now. Thanks for you help. It's still not as fast as it used to be but it is a whole lot better than it was before. I think virtual functions slowed it down a bit but I can live with that.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.