[haiku-commits] Re: r42080 - in haiku/trunk/src/apps/debugger: . model user_interface user_interface/gui user_interface/gui/inspector_window
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Fri, 10 Jun 2011 10:02:10 +0200
On 2011-06-10 at 03:58:40 [+0200], anevilyak@xxxxxxxxx wrote:
> Added: haiku/trunk/src/apps/debugger/TeamMemoryBlockManager.cpp
> ===================================================================
> --- haiku/trunk/src/apps/debugger/TeamMemoryBlockManager.cpp
> (rev 0)
> +++ haiku/trunk/src/apps/debugger/TeamMemoryBlockManager.cpp 2011-06-10
> 01:58:39 UTC (rev 42080)
[...]
> +TeamMemoryBlock*
> +TeamMemoryBlockManager::GetMemoryBlock(target_addr_t address)
> +{
> + AutoLocker<BLocker> lock(fLock);
> +
> + address &= ~B_PAGE_SIZE - 1;
> + MemoryBlockEntry* entry = fActiveBlocks->Lookup(address);
> + if (entry == NULL) {
> + TeamMemoryBlock* block = new(std::nothrow) TeamMemoryBlock(address,
> + this);
> + if (block == NULL)
> + return NULL;
> +
> + entry = new(std::nothrow) MemoryBlockEntry(block);
> + if (entry == NULL) {
> + delete block;
> + return NULL;
> + }
> +
> + fActiveBlocks->Insert(entry);
> + }
> +
> + int32 refCount = entry->block->AcquireReference();
> + if (refCount == 0) {
> + // this block already had its last reference released,
> + // move it to the dead list and retrieve a new one instead.
> + _MarkDeadBlock(address);
> + return GetMemoryBlock(address);
The recursion can be avoided by simply reordering the code:
lookup
if existing
get reference
unless last reference already released, return
move to dead list
allocate new
refCount isn't needed BTW.
> + }
> +
> + return entry->block;
> +}
[...]
> +void
> +TeamMemoryBlockManager::_MarkDeadBlock(target_addr_t address)
> +{
> + AutoLocker<BLocker> lock(fLock);
Already locked.
> + MemoryBlockEntry* entry = fActiveBlocks->Lookup(address);
> + if (entry != NULL) {
> + fActiveBlocks->Remove(entry);
> + fDeadBlocks->Insert(entry);
> + }
> +}
[...]
> Added: haiku/trunk/src/apps/debugger/TeamMemoryBlockManager.h
> ===================================================================
> --- haiku/trunk/src/apps/debugger/TeamMemoryBlockManager.h
> (rev 0)
> +++ haiku/trunk/src/apps/debugger/TeamMemoryBlockManager.h 2011-06-10
> 01:58:39 UTC (rev 42080)
> @@ -0,0 +1,50 @@
> +/*
> + * Copyright 2011, Rene Gollent, rene@xxxxxxxxxxxx
> + * Distributed under the terms of the MIT License.
> + */
> +#ifndef TEAM_MEMORY_BLOCK_MANAGER_H
> +#define TEAM_MEMORY_BLOCK_MANAGER_H
> +
> +
> +#include <Locker.h>
> +#include <Referenceable.h>
> +#include <util/OpenHashTable.h>
> +
> +#include "Types.h"
> +
Missing blank line.
> +struct MemoryBlockHashDefinition;
> +class TeamMemoryBlock;
> +
> +
> +class TeamMemoryBlockManager
> +{
> +public:
> + TeamMemoryBlockManager();
> + ~TeamMemoryBlockManager();
> +
> + status_t Init();
> +
> + TeamMemoryBlock* GetMemoryBlock(target_addr_t address);
> +
> +private:
> + struct Key;
> + struct MemoryBlockEntry;
> + struct MemoryBlockHashDefinition;
> + typedef BOpenHashTable<MemoryBlockHashDefinition> MemoryBlockTable;
> +
> +private:
> + void _Cleanup();
> + void _MarkDeadBlock(target_addr_t address);
> + void _RemoveBlock(target_addr_t address);
> +
> +private:
> + friend class TeamMemoryBlock;
> +
> +private:
> + BLocker fLock;
> + MemoryBlockTable* fActiveBlocks;
> + MemoryBlockTable* fDeadBlocks;
A list should be fully sufficient for the dead blocks. It's a bit unfortunate
that SinglyLinkedList requires a SinglyLinkedListLink. That has annoyed me
more than once.
> +};
> +
> +
> +#endif // TEAM_MEMORY_BLOCK_MANAGER_H
>
> Added: haiku/trunk/src/apps/debugger/model/TeamMemoryBlock.cpp
> ===================================================================
> --- haiku/trunk/src/apps/debugger/model/TeamMemoryBlock.cpp
> (rev 0)
> +++ haiku/trunk/src/apps/debugger/model/TeamMemoryBlock.cpp 2011-06-10
> 01:58:39 UTC (rev 42080)
[...]
> +void
> +TeamMemoryBlock::LastReferenceReleased()
> +{
> + fBlockManager->_RemoveBlock(fBaseAddress);
> +
> + BReferenceable::LastReferenceReleased();
The base class version is guaranteed to only call delete, so that could be
done directly as well. At any rate either the deletes in _RemoveBlock() or
the one here needs to go. I'd vote for keeping the one here.
> +}
> Added: haiku/trunk/src/apps/debugger/model/TeamMemoryBlock.h
> ===================================================================
> --- haiku/trunk/src/apps/debugger/model/TeamMemoryBlock.h
> (rev 0)
> +++ haiku/trunk/src/apps/debugger/model/TeamMemoryBlock.h 2011-06-10
> 01:58:39 UTC (rev 42080)
[...]
> +class TeamMemoryBlock : public BReferenceable {
> +public:
> + class Listener;
> +
> +public:
> + TeamMemoryBlock(target_addr_t baseAddress,
> + TeamMemoryBlockManager* manager);
> + ~TeamMemoryBlock();
> +
> + void AddListener(Listener* listener);
> + bool HasListener(Listener* listener);
> + void RemoveListener(Listener* listener);
> +
> + bool IsValid() const { return fValid; };
> + void MarkValid();
> + void Invalidate();
> +
> + target_addr_t BaseAddress() const { return
> fBaseAddress; };
> + uint8* Data() { return fData; };
> + size_t Size() const { return
> sizeof(fData); };
> +
> + void NotifyDataRetrieved();
> +
> +protected:
> + virtual void LastReferenceReleased();
> +
> +private:
> + typedef DoublyLinkedList<Listener> ListenerList;
> +
> +private:
> + bool fValid;
> + target_addr_t fBaseAddress;
> + uint8 fData[B_PAGE_SIZE];
> + ListenerList fListeners;
> + TeamMemoryBlockManager*
> + fBlockManager;
> + BLocker fLock;
It would be nicer from a design point of view to avoid the direct knowledge
of TeamMemoryBlockManager in TeamMemoryBlock by introducing a
TeamMemoryBlockOwner interface with a single callback.
> +};
CU, Ingo
Other related posts:
