Skip to content

Commit

Permalink
FBTF: Mark the Read methods in the IPC subsystem as noinline.
Browse files Browse the repository at this point in the history
This forces all the ReadParam template junk to expand once in the *_messages.cc
file, instead of at every Read() call site. Without the compiler-specific
annotation, this builds and links in debug mode, but doesn't link in release
mode because the individual Read() methods generated were inlined into the
subclass Log() methods, causing disaster on the release builders, but not on
the trybots or locally.

BUG=51411
TEST=none

Review URL: http://codereview.chromium.org/3160008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56081 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
erg@google.com committed Aug 13, 2010
1 parent 487cf3d commit 55b4e21
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 32 deletions.
6 changes: 3 additions & 3 deletions chrome/chrome_common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,9 @@
'common/service_messages.cc',
'common/service_messages.h',
'common/services_messages_internal.h',
'common/service_process_type.h',
'common/service_process_util.cc',
'common/service_process_util.h',
'common/service_process_type.h',
'common/service_process_util.cc',
'common/service_process_util.h',
'common/socket_stream_dispatcher.cc',
'common/socket_stream_dispatcher.h',
'common/spellcheck_common.cc',
Expand Down
2 changes: 1 addition & 1 deletion ipc/ipc_message_impl_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
\
void msg_class::Log(const Message* msg, std::wstring* l) { \
if (msg->is_sync()) { \
SendParam p; \
TupleTypes<SendParam>::ValueTuple p; \
if (ReadSendParam(msg, &p)) \
IPC::LogParam(p, l); \
\
Expand Down
44 changes: 20 additions & 24 deletions ipc/ipc_message_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,21 @@
#include "base/utf_string_conversions.h"
#include "ipc/ipc_sync_message.h"

#if defined(COMPILER_GCC)
// GCC "helpfully" tries to inline template methods in release mode. Except we
// want the majority of the template junk being expanded once in the
// implementation file (and only provide the definitions in
// ipc_message_utils_impl.h in those files) and exported, instead of expanded
// at every call site. Special note: GCC happily accepts the attribute before
// the method declaration, but only acts on it if it is after.
#define IPC_MSG_NOINLINE __attribute__((noinline));
#elif defined(COMPILER_MSVC)
// MSVC++ doesn't do this.
#define IPC_MSG_NOINLINE
#else
#error "Please add the noinline property for your new compiler here."
#endif

// Used by IPC_BEGIN_MESSAGES so that each message class starts from a unique
// base. Messages have unique IDs across channels in order for the IPC logging
// code to figure out the message class from its ID.
Expand Down Expand Up @@ -960,16 +975,7 @@ class MessageWithTuple : public Message {
// those translation units.
MessageWithTuple(int32 routing_id, uint32 type, const RefParam& p);

// TODO(erg): Migrate this method into ipc_message_utils_impl.h once I figure
// out why just having the template in that file and the forward declaration
// here breaks the release build.
static bool Read(const Message* msg, Param* p) {
void* iter = NULL;
if (ReadParam(msg, &iter, p))
return true;
NOTREACHED() << "Error deserializing message " << msg->type();
return false;
}
static bool Read(const Message* msg, Param* p) IPC_MSG_NOINLINE;

// Generic dispatcher. Should cover most cases.
template<class T, class Method>
Expand Down Expand Up @@ -1160,20 +1166,10 @@ class MessageWithReply : public SyncMessage {

MessageWithReply(int32 routing_id, uint32 type,
const RefSendParam& send, const ReplyParam& reply);

// TODO(erg): Migrate these ReadSendParam/ReadReplyParam() methods to
// ipc_message_utils_impl.h once I figure out how to get the linkage correct
// in the release builds.
static bool ReadSendParam(const Message* msg, SendParam* p) {
void* iter = SyncMessage::GetDataIterator(msg);
return ReadParam(msg, &iter, p);
}

static bool ReadReplyParam(const Message* msg,
typename TupleTypes<ReplyParam>::ValueTuple* p) {
void* iter = SyncMessage::GetDataIterator(msg);
return ReadParam(msg, &iter, p);
}
static bool ReadSendParam(const Message* msg, SendParam* p) IPC_MSG_NOINLINE;
static bool ReadReplyParam(
const Message* msg,
typename TupleTypes<ReplyParam>::ValueTuple* p) IPC_MSG_NOINLINE;

template<class T, class Method>
static bool Dispatch(const Message* msg, T* obj, Method func) {
Expand Down
25 changes: 21 additions & 4 deletions ipc/ipc_message_utils_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,14 @@ MessageWithTuple<ParamType>::MessageWithTuple(
WriteParam(this, p);
}

// TODO(erg): Migrate MessageWithTuple<ParamType>::Read() here once I figure
// out why having the definition here doesn't export the symbols.
template <class ParamType>
bool MessageWithTuple<ParamType>::Read(const Message* msg, Param* p) {
void* iter = NULL;
if (ReadParam(msg, &iter, p))
return true;
NOTREACHED() << "Error deserializing message " << msg->type();
return false;
}

// We can't migrate the template for Log() to MessageWithTuple, because each
// subclass needs to have Log() to call Read(), which instantiates the above
Expand All @@ -35,8 +41,19 @@ MessageWithReply<SendParamType, ReplyParamType>::MessageWithReply(
WriteParam(this, send);
}

// TODO(erg): Migrate ReadSendParam()/ReadReplyParam() here when we can force
// the visibility/linkage.
template <class SendParamType, class ReplyParamType>
bool MessageWithReply<SendParamType, ReplyParamType>::ReadSendParam(
const Message* msg, SendParam* p) {
void* iter = SyncMessage::GetDataIterator(msg);
return ReadParam(msg, &iter, p);
}

template <class SendParamType, class ReplyParamType>
bool MessageWithReply<SendParamType, ReplyParamType>::ReadReplyParam(
const Message* msg, typename TupleTypes<ReplyParam>::ValueTuple* p) {
void* iter = SyncMessage::GetDataIterator(msg);
return ReadParam(msg, &iter, p);
}

} // namespace IPC

Expand Down

0 comments on commit 55b4e21

Please sign in to comment.