Some refactoring: Separated the profile result collection from the

thread management. There's now a ThreadProfileResult (abstract base
class) object associated with a Thread object. Currently there's only
one (currently misnamed) derived class AbstractThreadProfileResult, but
some more refactoring will make the purpose clearer.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@27780 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/bin/debug/profile/Thread.cpp

@@ -124,26 +124,25 @@ Thread::Thread(const thread_info& info, Team* team)
 	fTeam(team),
 	fSampleArea(-1),
 	fSamples(NULL),
-	fImages(),
+	fProfileResult(NULL)
-	fOldImages(),
-	fTotalTicks(0),
-	fUnkownTicks(0),
-	fDroppedTicks(0),
-	fInterval(1),
-	fTotalSampleCount(0)
 {
 }
 
 
 Thread::~Thread()
 {
-	while (ThreadImage* image = fImages.RemoveHead())
-		delete image;
-	while (ThreadImage* image = fOldImages.RemoveHead())
-		delete image;
-
 	if (fSampleArea >= 0)
 		delete_area(fSampleArea);
+
+	delete fProfileResult;
+}
+
+
+void
+Thread::SetProfileResult(ThreadProfileResult* result)
+{
+	delete fProfileResult;
+	fProfileResult = result;
 }
 
 
@@ -167,12 +166,131 @@ Thread::SetSampleArea(area_id area, addr_t* samples)
 void
 Thread::SetInterval(bigtime_t interval)
 {
-	fInterval = interval;
+	fProfileResult->SetInterval(interval);
 }
 
 
 status_t
 Thread::AddImage(Image* image)
+{
+	return fProfileResult->AddImage(image);
+}
+
+
+void
+Thread::AddSamples(int32 count, int32 dropped, int32 stackDepth,
+	bool variableStackDepth, int32 event)
+{
+	fProfileResult->SynchronizeImages(event);
+
+	if (variableStackDepth) {
+		addr_t* samples = fSamples;
+
+		while (count > 0) {
+			addr_t sampleCount = *(samples++);
+
+			if (sampleCount >= B_DEBUG_PROFILE_EVENT_BASE) {
+				int32 eventParameterCount
+					= sampleCount & B_DEBUG_PROFILE_EVENT_PARAMETER_MASK;
+				if (sampleCount == B_DEBUG_PROFILE_IMAGE_EVENT) {
+					fProfileResult->SynchronizeImages((int32)samples[0]);
+				} else {
+					fprintf(stderr, "unknown profile event: %#lx\n",
+						sampleCount);
+				}
+
+				samples += eventParameterCount;
+				count -= eventParameterCount + 1;
+				continue;
+			}
+
+			fProfileResult->AddSamples(samples, sampleCount);
+
+			samples += sampleCount;
+			count -= sampleCount + 1;
+		}
+	} else {
+		count = count / stackDepth * stackDepth;
+
+		for (int32 i = 0; i < count; i += stackDepth)
+			fProfileResult->AddSamples(fSamples + i, stackDepth);
+	}
+
+	fProfileResult->AddDroppedTicks(dropped);
+}
+
+
+void
+Thread::PrintResults() const
+{
+	fProfileResult->PrintResults();
+}
+
+
+// #pragma mark - ThreadProfileResult
+
+
+ThreadProfileResult::ThreadProfileResult()
+	:
+	fThread(NULL),
+	fInterval(1)
+{
+}
+
+
+ThreadProfileResult::~ThreadProfileResult()
+{
+}
+
+
+status_t
+ThreadProfileResult::Init(Thread* thread)
+{
+	fThread = thread;
+	return B_OK;
+}
+
+
+void
+ThreadProfileResult::SetInterval(bigtime_t interval)
+{
+	fInterval = interval;
+}
+
+
+// #pragma mark - AbstractThreadProfileResult
+
+AbstractThreadProfileResult::AbstractThreadProfileResult()
+	:
+	fImages(),
+	fNewImages(),
+	fOldImages(),
+	fTotalTicks(0),
+	fUnkownTicks(0),
+	fDroppedTicks(0),
+	fTotalSampleCount(0)
+{
+}
+
+
+AbstractThreadProfileResult::~AbstractThreadProfileResult()
+{
+	while (ThreadImage* image = fImages.RemoveHead())
+		delete image;
+	while (ThreadImage* image = fOldImages.RemoveHead())
+		delete image;
+}
+
+
+status_t
+AbstractThreadProfileResult::Init(Thread* thread)
+{
+	return ThreadProfileResult::Init(thread);
+}
+
+
+status_t
+AbstractThreadProfileResult::AddImage(Image* image)
 {
 	ThreadImage* threadImage = new(std::nothrow) ThreadImage(image);
 	if (threadImage == NULL)
@@ -184,31 +302,41 @@ Thread::AddImage(Image* image)
 		return error;
 	}
 
-	fImages.Add(threadImage);
+	fNewImages.Add(threadImage);
 
 	return B_OK;
 }
 
 
 void
-Thread::ImageRemoved(Image* image)
+AbstractThreadProfileResult::SynchronizeImages(int32 event)
 {
+	// remove obsolete images
 	ImageList::Iterator it = fImages.GetIterator();
-	while (ThreadImage* threadImage = it.Next()) {
+	while (ThreadImage* image = it.Next()) {
-		if (threadImage->GetImage() == image) {
+		int32 deleted = image->GetImage()->DeletionEvent();
-			fImages.Remove(threadImage);
+		if (deleted >= 0 && event >= deleted) {
-			if (threadImage->TotalHits() > 0)
+			it.Remove();
-				fOldImages.Add(threadImage);
+			if (image->TotalHits() > 0)
+				fOldImages.Add(image);
 			else
-				delete threadImage;
+				delete image;
-			return;
+		}
+	}
+
+	// add new images
+	it = fNewImages.GetIterator();
+	while (ThreadImage* image = it.Next()) {
+		if (image->GetImage()->CreationEvent() >= event) {
+			it.Remove();
+			fImages.Add(image);
 		}
 	}
 }
 
 
 ThreadImage*
-Thread::FindImage(addr_t address) const
+AbstractThreadProfileResult::FindImage(addr_t address) const
 {
 	ImageList::ConstIterator it = fImages.GetIterator();
 	while (ThreadImage* image = it.Next()) {
@@ -220,127 +348,75 @@ Thread::FindImage(addr_t address) const
 
 
 void
-Thread::AddSamples(int32 count, int32 dropped, int32 stackDepth,
+AbstractThreadProfileResult::AddSamples(addr_t* samples, int32 sampleCount)
-	bool variableStackDepth, int32 event)
 {
-	_RemoveObsoleteImages(event);
+	if (gOptions.analyze_full_stack) {
+		// Sort the samples. This way hits of the same symbol are
+		// successive and we can avoid incrementing the hit count of the
+		// same symbol twice. Same for images.
+		std::sort(samples, samples + sampleCount);
 
-	// Temporarily remove images that have been created after the given
+		int32 unknownSamples = 0;
-	// event.
+		ThreadImage* previousImage = NULL;
-	ImageList newImages;
+		int32 previousSymbol = -1;
-	ImageList::Iterator it = fImages.GetIterator();
-	while (ThreadImage* image = it.Next()) {
-		if (image->GetImage()->CreationEvent() > event) {
-			it.Remove();
-			newImages.Add(image);
-		}
-	}
 
-	if (variableStackDepth) {
+		for (int32 i = 0; i < sampleCount; i++) {
-		// Variable stack depth means we have full caller stack analyzes
+			addr_t address = samples[i];
-		// enabled.
+			ThreadImage* image = FindImage(address);
-		int32 totalSampleCount = 0;
+			int32 symbol = -1;
-		int32 tickCount = 0;
+			if (image != NULL) {
-		addr_t* samples = fSamples;
+				symbol = image->GetImage()->FindSymbol(address);
+				if (symbol < 0) {
+					// TODO: Count unknown image hits?
+				} else if (symbol != previousSymbol)
+					image->AddSymbolHit(symbol);
 
-		while (count > 0) {
+				if (image != previousImage)
-			addr_t sampleCount = *(samples++);
+					image->AddImageHit();
+			} else
+				unknownSamples++;
 
-			if (sampleCount >= B_DEBUG_PROFILE_EVENT_BASE) {
+			previousImage = image;
-				int32 eventParameterCount
+			previousSymbol = symbol;
-					= sampleCount & B_DEBUG_PROFILE_EVENT_PARAMETER_MASK;
-				if (sampleCount == B_DEBUG_PROFILE_IMAGE_EVENT) {
-					int32 imageEvent = (int32)samples[0];
-					_RemoveObsoleteImages(imageEvent);
-					_AddNewImages(newImages, imageEvent);
-				} else {
-					fprintf(stderr, "unknown profile event: %#lx\n",
-						sampleCount);
-				}
-
-				samples += eventParameterCount;
-				count -= eventParameterCount + 1;
-				continue;
-			}
-
-			// Sort the samples. This way hits of the same symbol are
-			// successive and we can avoid incrementing the hit count of the
-			// same symbol twice. Same for images.
-			std::sort(samples, samples + sampleCount);
-
-			int32 unknownSamples = 0;
-			ThreadImage* previousImage = NULL;
-			int32 previousSymbol = -1;
-
-			for (uint32 i = 0; i < sampleCount; i++) {
-				addr_t address = samples[i];
-				ThreadImage* image = FindImage(address);
-				int32 symbol = -1;
-				if (image != NULL) {
-					symbol = image->GetImage()->FindSymbol(address);
-					if (symbol < 0) {
-						// TODO: Count unknown image hits?
-					} else if (symbol != previousSymbol)
-						image->AddSymbolHit(symbol);
-
-					if (image != previousImage)
-						image->AddImageHit();
-				} else
-					unknownSamples++;
-
-				previousImage = image;
-				previousSymbol = symbol;
-			}
-
-			if (unknownSamples == (int32)sampleCount)
-				fUnkownTicks++;
-
-			samples += sampleCount;
-			count -= sampleCount + 1;
-			tickCount++;
-			totalSampleCount += sampleCount;
 		}
 
-		fTotalTicks += tickCount;
+		if (unknownSamples == sampleCount)
-		fTotalSampleCount += totalSampleCount;
+			fUnkownTicks++;
 	} else {
-		count = count / stackDepth * stackDepth;
+		ThreadImage* image = NULL;
-
+		for (int32 k = 0; k < sampleCount; k++) {
-		for (int32 i = 0; i < count; i += stackDepth) {
+			addr_t address = samples[k];
-			ThreadImage* image = NULL;
+			image = FindImage(address);
-			for (int32 k = 0; k < stackDepth; k++) {
+			if (image != NULL) {
-				addr_t address = fSamples[i + k];
+				image->AddHit(address);
-				image = FindImage(address);
+				break;
-				if (image != NULL) {
-					image->AddHit(address);
-					break;
-				}
 			}
-
-			if (image == NULL)
-				fUnkownTicks++;
 		}
 
-		fTotalTicks += count / stackDepth;
+		if (image == NULL)
+			fUnkownTicks++;
 	}
 
-	fDroppedTicks += dropped;
+	fTotalTicks++;
-
+	fTotalSampleCount += sampleCount;
-	// re-add the new images
-	fImages.MoveFrom(&newImages);
-
-	_SynchronizeImages();
 }
 
 
 void
-Thread::PrintResults() const
+AbstractThreadProfileResult::AddDroppedTicks(int32 dropped)
+{
+	fDroppedTicks += dropped;
+}
+
+
+void
+AbstractThreadProfileResult::PrintResults()
 {
 	// count images and symbols
 	int32 imageCount = 0;
 	int32 symbolCount = 0;
 
-	ImageList::ConstIterator it = fOldImages.GetIterator();
+	ImageList::Iterator it = fOldImages.GetIterator();
 	while (ThreadImage* image = it.Next()) {
 		if (image->TotalHits() > 0) {
 			imageCount++;
@@ -398,7 +474,7 @@ Thread::PrintResults() const
 
 	int64 totalTicks = fTotalTicks;
 	fprintf(gOptions.output, "\nprofiling results for thread \"%s\" "
-		"(%ld):\n", Name(), ID());
+		"(%ld):\n", fThread->Name(), fThread->ID());
 	fprintf(gOptions.output, "  tick interval:  %lld us\n", fInterval);
 	fprintf(gOptions.output, "  total ticks:    %lld (%lld us)\n",
 		totalTicks, totalTicks * fInterval);
@@ -447,71 +523,3 @@ Thread::PrintResults() const
 		fprintf(gOptions.output, "  no functions were hit\n");
 }
 
-
-void
-Thread::_SynchronizeImages()
-{
-	const BObjectList<Image>& teamImages = fTeam->Images();
-
-	// remove obsolete images
-	ImageList::Iterator it = fImages.GetIterator();
-	while (ThreadImage* image = it.Next()) {
-		if (fTeam->FindImage(image->ID()) == NULL) {
-			it.Remove();
-			if (image->TotalHits() > 0)
-				fOldImages.Add(image);
-			else
-				delete image;
-		}
-	}
-
-	// add new images
-	for (int32 i = 0; Image* image = teamImages.ItemAt(i); i++) {
-		if (_FindImageByID(image->ID()) == NULL)
-			AddImage(image);
-	}
-}
-
-
-void
-Thread::_AddNewImages(ImageList& newImages, int32 event)
-{
-	ImageList::Iterator it = newImages.GetIterator();
-	while (ThreadImage* image = it.Next()) {
-		if (image->GetImage()->CreationEvent() >= event) {
-			it.Remove();
-			fImages.Add(image);
-		}
-	}
-}
-
-
-void
-Thread::_RemoveObsoleteImages(int32 event)
-{
-	// remove obsolete images
-	ImageList::Iterator it = fImages.GetIterator();
-	while (ThreadImage* image = it.Next()) {
-		int32 deleted = image->GetImage()->DeletionEvent();
-		if (deleted >= 0 && event >= deleted) {
-			it.Remove();
-			if (image->TotalHits() > 0)
-				fOldImages.Add(image);
-			else
-				delete image;
-		}
-	}
-}
-
-
-ThreadImage*
-Thread::_FindImageByID(image_id id) const
-{
-	ImageList::ConstIterator it = fImages.GetIterator();
-	while (ThreadImage* image = it.Next()) {
-		if (image->ID() == id)
-			return image;
-	}
-
-	return NULL;
-}

src/bin/debug/profile/Thread.h

@@ -12,6 +12,7 @@
 
 class Team;
 class ThreadImage;
+class ThreadProfileResult;
 
 
 class Thread : public DoublyLinkedListLinkImpl<Thread> {
@@ -24,14 +25,15 @@ public:
 	inline	addr_t*				Samples() const;
 	inline	Team*				GetTeam() const;
 
+	inline	ThreadProfileResult* ProfileResult() const;
+			void				SetProfileResult(ThreadProfileResult* result);
+
 			void				UpdateInfo();
 
 			void				SetSampleArea(area_id area, addr_t* samples);
 			void				SetInterval(bigtime_t interval);
 
 			status_t			AddImage(Image* image);
-			void				ImageRemoved(Image* image);
-			ThreadImage*		FindImage(addr_t address) const;
 
 			void				AddSamples(int32 count, int32 dropped,
 									int32 stackDepth, bool variableStackDepth,
@@ -41,25 +43,64 @@ public:
 private:
 	typedef DoublyLinkedList<ThreadImage>	ImageList;
 
-			void				_SynchronizeImages();
-
-			void				_AddNewImages(ImageList& newImages,
-									int32 event);
-			void				_RemoveObsoleteImages(int32 event);
-
-			ThreadImage*		_FindImageByID(image_id id) const;
-
 private:
 			thread_info			fInfo;
 			::Team*				fTeam;
 			area_id				fSampleArea;
 			addr_t*				fSamples;
+			ThreadProfileResult* fProfileResult;
+};
+
+
+class ThreadProfileResult {
+public:
+								ThreadProfileResult();
+	virtual						~ThreadProfileResult();
+
+	virtual	status_t			Init(Thread* thread);
+
+			void				SetInterval(bigtime_t interval);
+
+	virtual	status_t			AddImage(Image* image) = 0;
+	virtual	void				SynchronizeImages(int32 event) = 0;
+
+	virtual	void				AddSamples(addr_t* samples,
+									int32 sampleCount) = 0;
+	virtual	void				AddDroppedTicks(int32 dropped) = 0;
+	virtual	void				PrintResults() = 0;
+
+protected:
+			Thread*				fThread;
+			bigtime_t			fInterval;
+};
+
+
+class AbstractThreadProfileResult : public ThreadProfileResult {
+public:
+								AbstractThreadProfileResult();
+	virtual						~AbstractThreadProfileResult();
+
+	virtual	status_t			Init(Thread* thread);
+
+	virtual	status_t			AddImage(Image* image);
+	virtual	void				SynchronizeImages(int32 event);
+
+			ThreadImage*		FindImage(addr_t address) const;
+
+	virtual	void				AddSamples(addr_t* samples,
+									int32 sampleCount);
+	virtual	void				AddDroppedTicks(int32 dropped);
+	virtual	void				PrintResults();
+
+protected:
+	typedef DoublyLinkedList<ThreadImage>	ImageList;
+
 			ImageList			fImages;
+			ImageList			fNewImages;
 			ImageList			fOldImages;
 			int64				fTotalTicks;
 			int64				fUnkownTicks;
 			int64				fDroppedTicks;
-			bigtime_t			fInterval;
 			int64				fTotalSampleCount;
 };
 
@@ -95,4 +136,11 @@ Thread::GetTeam() const
 }
 
 
+ThreadProfileResult*
+Thread::ProfileResult() const
+{
+	return fProfileResult;
+}
+
+
 #endif	// THREAD_H

src/bin/debug/profile/profile.cpp

@@ -123,6 +123,12 @@ public:
 		if (thread == NULL)
 			return B_NO_MEMORY;
 
+		error = _CreateThreadProfileResult(thread);
+		if (error != B_OK) {
+			delete thread;
+			return error;
+		}
+
 		error = team->InitThread(thread);
 		if (error != B_OK) {
 			delete thread;
@@ -165,6 +171,24 @@ public:
 		return NULL;
 	}
 
+private:
+	status_t _CreateThreadProfileResult(Thread* thread)
+	{
+		ThreadProfileResult* profileResult
+			= new(std::nothrow) AbstractThreadProfileResult;
+		if (profileResult == NULL)
+			return B_NO_MEMORY;
+
+		status_t error = profileResult->Init(thread);
+		if (error != B_OK) {
+			delete profileResult;
+			return error;
+		}
+
+		thread->SetProfileResult(profileResult);
+		return B_OK;
+	}
+
 private:
 	BObjectList<Team>				fTeams;
 	BObjectList<Thread>				fThreads;