From b862e4a9956876f1edd246de757f7cd10dd47281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Boisvert?= Date: Tue, 5 Nov 2013 15:51:53 -0500 Subject: [PATCH] fix a regression introduced in a01f97eae41bcd759bfc521d84053552cf38d521 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Internal buffers must be aligned on 8 bytes because a lot of framework code assumes that this alignment is there. Signed-off-by: Sébastien Boisvert --- RayPlatform/communication/Message.cpp | 12 +-- RayPlatform/communication/Message.h | 76 +++++++++++-------- RayPlatform/communication/MessagesHandler.cpp | 6 +- RayPlatform/core/ComputeCore.cpp | 74 ++++++++++++++---- 4 files changed, 111 insertions(+), 57 deletions(-) diff --git a/RayPlatform/communication/Message.cpp b/RayPlatform/communication/Message.cpp index 1159a60e..1ec3e8f6 100644 --- a/RayPlatform/communication/Message.cpp +++ b/RayPlatform/communication/Message.cpp @@ -43,12 +43,6 @@ using namespace std; * */ -#define MESSAGE_META_DATA_ACTOR_SOURCE (0 * sizeof(int) ) -#define MESSAGE_META_DATA_ACTOR_DESTINATION (1 * sizeof(int) ) -#define MESSAGE_META_DATA_ROUTE_SOURCE (2 * sizeof(int) ) -#define MESSAGE_META_DATA_ROUTE_DESTINATION (3 * sizeof(int) ) -#define MESSAGE_META_DATA_CHECKSUM (4 * sizeof(int) ) - Message::Message() { @@ -72,8 +66,10 @@ void Message::initialize() { Message::~Message() { } -/** buffer must be allocated or else it will CORE DUMP. */ -Message::Message(MessageUnit*b,int c,Rank dest,MessageTag tag,Rank source){ +/** + * buffer must be allocated or else it will CORE DUMP. + */ +Message::Message(void*b,int c,Rank dest,MessageTag tag,Rank source){ initialize(); diff --git a/RayPlatform/communication/Message.h b/RayPlatform/communication/Message.h index ca992629..37ad25ec 100644 --- a/RayPlatform/communication/Message.h +++ b/RayPlatform/communication/Message.h @@ -1,48 +1,57 @@ /* - RayPlatform: a message-passing development framework - Copyright (C) 2010, 2011, 2012 Sébastien Boisvert +RayPlatform: a message-passing development framework +Copyright (C) 2010, 2011, 2012 Sébastien Boisvert - http://github.com/sebhtml/RayPlatform +http://github.com/sebhtml/RayPlatform - This program is free software: you can redistribute it and/or modify - it under the terms of the GNU Lesser General Public License as published by - the Free Software Foundation, version 3 of the License. +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation, version 3 of the License. - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU Lesser General Public License for more details. +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. - You have received a copy of the GNU Lesser General Public License - along with this program (lgpl-3.0.txt). - see +You have received a copy of the GNU Lesser General Public License +along with this program (lgpl-3.0.txt). +see */ #ifndef _Message_H #define _Message_H +#define MESSAGE_META_DATA_ACTOR_SOURCE 0 +#define MESSAGE_META_DATA_ACTOR_DESTINATION ( MESSAGE_META_DATA_ACTOR_SOURCE + sizeof(int) ) +#define MESSAGE_META_DATA_ROUTE_SOURCE ( MESSAGE_META_DATA_ACTOR_DESTINATION + sizeof(int) ) +#define MESSAGE_META_DATA_ROUTE_DESTINATION ( MESSAGE_META_DATA_ROUTE_SOURCE + sizeof(int)) +#define MESSAGE_META_DATA_CHECKSUM ( MESSAGE_META_DATA_ROUTE_DESTINATION + sizeof(int)) +#define MESSAGE_META_DATA_SIZE ( MESSAGE_META_DATA_CHECKSUM + sizeof(uint32_t) ) + + + #include #include /** - * In Ray, every message is a Message. - * the inbox and the outbox are arrays of Message's - * All the code in Ray utilise Message to communicate information. - * MPI_Datatype is always MPI_UNSIGNED_LONG_LONG - * m_count is >=0 and <= MAXIMUM_MESSAGE_SIZE_IN_BYTES/sizeof(uint64_t) - * (default is 4000/8 = 500 ). - * - * in the buffer: - * - * Application payload (works) - * int source actor (works) - * int destination actor (works) - * int routing source (works) - * int routing destination (works) - * int checksum (probably broken) - * +* In Ray, every message is a Message. +* the inbox and the outbox are arrays of Message's +* All the code in Ray utilise Message to communicate information. +* MPI_Datatype is always MPI_UNSIGNED_LONG_LONG +* m_count is >=0 and <= MAXIMUM_MESSAGE_SIZE_IN_BYTES/sizeof(uint64_t) +* (default is 4000/8 = 500 ). +* +* in the buffer: +* +* Application payload (works) +* int source actor (works) +* int destination actor (works) +* int routing source (works) +* int routing destination (works) +* int checksum (probably broken) +* * The offset (see Message.cpp) are relative to the number of bytes *BEFORE* * adding this metadata. * Note that the m_count is the number of MessageUnit for the message @@ -116,7 +125,14 @@ class Message{ public: Message(); ~Message(); - Message(MessageUnit * b,int c,Rank dest,MessageTag tag,Rank source); + + /** + * numberOf8Bytes is there for historical reasons. + * You can call setNumberOfBytes afterward to set the number of bytes anyway. + * + */ + Message(void* buffer, int numberOf8Bytes, Rank destination, MessageTag tag, + Rank source); MessageUnit *getBuffer(); char * getBufferBytes(); int getCount() const; diff --git a/RayPlatform/communication/MessagesHandler.cpp b/RayPlatform/communication/MessagesHandler.cpp index 8c52129e..23cc498c 100644 --- a/RayPlatform/communication/MessagesHandler.cpp +++ b/RayPlatform/communication/MessagesHandler.cpp @@ -656,7 +656,7 @@ void MessagesHandler::probeAndRead(Rank source,MessageTag tag, assert(count >= 0); #endif - MessageUnit staticBuffer[MAXIMUM_MESSAGE_SIZE_IN_BYTES/sizeof(MessageUnit)+2]; + char staticBuffer[MAXIMUM_MESSAGE_SIZE_IN_BYTES + MESSAGE_META_DATA_SIZE]; MPI_Recv(staticBuffer,count,datatype,actualSource,actualTag,MPI_COMM_WORLD,&status); @@ -1124,9 +1124,9 @@ void MessagesHandler::receiveMessages(StaticVector*inbox,RingAllocator*inboxAllo assert(count >= 0); #endif - MessageUnit*incoming=NULL; + char * incoming=NULL; if(count > 0){ - incoming=(MessageUnit*)inboxAllocator->allocate(count * sizeof(char)); + incoming=(char*)inboxAllocator->allocate(count * sizeof(char)); } MPI_Recv(incoming,count,datatype,actualSource,actualTag,MPI_COMM_WORLD,&status); diff --git a/RayPlatform/core/ComputeCore.cpp b/RayPlatform/core/ComputeCore.cpp index 470d0ea3..8d11e271 100644 --- a/RayPlatform/core/ComputeCore.cpp +++ b/RayPlatform/core/ComputeCore.cpp @@ -671,7 +671,7 @@ void ComputeCore::sendMessages(){ assert(forceMemoryReallocation || aMessage->getCount()==0||numberOfMessages==m_size); #endif - char * buffer=(char*)(MessageUnit*)m_outboxAllocator.allocate(MAXIMUM_MESSAGE_SIZE_IN_BYTES); + char * buffer=(char*)m_outboxAllocator.allocate(MAXIMUM_MESSAGE_SIZE_IN_BYTES); #ifdef ASSERT assert(buffer!=NULL); @@ -739,7 +739,7 @@ void ComputeCore::sendMessages(){ // we need a buffer to store the actor metadata if(message->getBuffer() == NULL) { void * buffer = m_outboxAllocator.allocate(0); - message->setBuffer((MessageUnit*)buffer); + message->setBuffer(buffer); } /* @@ -889,8 +889,8 @@ void ComputeCore::verifyMessageChecksums(){ assert(message->getNumberOfBytes() >= 0); #endif - // the last MessageUnit contains the crc32. - // note that the CRC32 feature only works for MessageUnit at least + // the last Message Unit contains the crc32. + // note that the CRC32 feature only works for Message Unit at least // 32 bits. int numberOfBytes = m_inbox.at(i)->getNumberOfBytes(); @@ -1056,7 +1056,7 @@ void ComputeCore::constructor(int argc,char**argv,int miniRankNumber,int numberO #endif // checksum calculation is only tested - // for cases with sizeof(MessageUnit)>=4 bytes + // for cases with sizeof(Message Unit)>=4 bytes m_routerIsEnabled=false; @@ -1183,33 +1183,75 @@ void ComputeCore::configureEngine() { cout<<"[ComputeCore] the outbox capacity is "< it is likely because of the underlying system that evolved to + // use 64 bits per message unit. + + bool align = true; + //align = false; + + int base = 8; + int remainder = headerSize % base; + + if(align && remainder != 0) { + + int toAdd = base - remainder; + + headerSize += toAdd; + } + + cout << " headerSize (with padding) " << headerSize << endl; + + maximumMessageSizeInBytes += headerSize; } + cout << "DEBUG maximumMessageSizeInBytes " << maximumMessageSizeInBytes << endl; + +#ifdef CONFIG_ASSERT + assert(maximumMessageSizeInBytes >= MAXIMUM_MESSAGE_SIZE_IN_BYTES); + assert(maximumMessageSizeInBytes >= MAXIMUM_MESSAGE_SIZE_IN_BYTES + 20);// we need 20 bytes for storing message header +#endif + m_inboxAllocator.constructor(m_maximumAllocatedInboxBuffers, - maximumMessageSizeInByte, + maximumMessageSizeInBytes, "RAY_MALLOC_TYPE_INBOX_ALLOCATOR",false); m_outboxAllocator.constructor(m_maximumAllocatedOutboxBuffers, - maximumMessageSizeInByte, + maximumMessageSizeInBytes, "RAY_MALLOC_TYPE_OUTBOX_ALLOCATOR",false); if(m_miniRanksAreEnabled){ @@ -1219,18 +1261,18 @@ void ComputeCore::configureEngine() { #endif m_bufferedInboxAllocator.constructor(m_maximumAllocatedOutboxBuffers, - maximumMessageSizeInByte, + maximumMessageSizeInBytes, "RAY_MALLOC_TYPE_INBOX_ALLOCATOR/Buffered",false); m_bufferedOutboxAllocator.constructor(m_maximumAllocatedOutboxBuffers, - maximumMessageSizeInByte, + maximumMessageSizeInBytes, "RAY_MALLOC_TYPE_OUTBOX_ALLOCATOR/Buffered",false); } #ifdef CONFIG_DEBUG_CORE - cout<<"[ComputeCore] allocated "<