From a4591b0475631b1baa3feb1e29b84ba318bb4147 Mon Sep 17 00:00:00 2001 From: Chris Busbey Date: Tue, 27 Sep 2016 09:36:33 -0500 Subject: [PATCH 1/2] removes unnecessary string alloc in logging fix messages --- file_log.go | 4 ++-- file_log_test.go | 4 ++-- log.go | 4 ++-- null_log.go | 6 +++--- screen_log.go | 4 ++-- session.go | 2 +- session_state.go | 2 +- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/file_log.go b/file_log.go index 536792a74..e385a3d29 100644 --- a/file_log.go +++ b/file_log.go @@ -14,11 +14,11 @@ type fileLog struct { messageLogger *log.Logger } -func (l fileLog) OnIncoming(msg string) { +func (l fileLog) OnIncoming(msg []byte) { l.messageLogger.Print(msg) } -func (l fileLog) OnOutgoing(msg string) { +func (l fileLog) OnOutgoing(msg []byte) { l.messageLogger.Print(msg) } diff --git a/file_log_test.go b/file_log_test.go index 6f978d06a..b807dca42 100644 --- a/file_log_test.go +++ b/file_log_test.go @@ -107,7 +107,7 @@ func TestFileLog_Append(t *testing.T) { messageScanner := bufio.NewScanner(messageLogFile) eventScanner := bufio.NewScanner(eventLogFile) - helper.Log.OnIncoming("incoming") + helper.Log.OnIncoming([]byte("incoming")) if !messageScanner.Scan() { t.Error("Unexpected EOF") } @@ -118,7 +118,7 @@ func TestFileLog_Append(t *testing.T) { } newHelper := newFileLogHelper(t) - newHelper.Log.OnIncoming("incoming") + newHelper.Log.OnIncoming([]byte("incoming")) if !messageScanner.Scan() { t.Error("Unexpected EOF") } diff --git a/log.go b/log.go index 78f3e7d41..6527e92dc 100644 --- a/log.go +++ b/log.go @@ -3,10 +3,10 @@ package quickfix //Log is a generic interface for logging FIX messages and events. type Log interface { //log incoming fix message - OnIncoming(string) + OnIncoming([]byte) //log outgoing fix message - OnOutgoing(string) + OnOutgoing([]byte) //log fix event OnEvent(string) diff --git a/null_log.go b/null_log.go index 10ff2c295..3d6f02ba1 100644 --- a/null_log.go +++ b/null_log.go @@ -2,9 +2,9 @@ package quickfix type nullLog struct{} -func (l nullLog) OnIncoming(s string) {} -func (l nullLog) OnOutgoing(s string) {} -func (l nullLog) OnEvent(s string) {} +func (l nullLog) OnIncoming([]byte) {} +func (l nullLog) OnOutgoing([]byte) {} +func (l nullLog) OnEvent(string) {} func (l nullLog) OnEventf(format string, a ...interface{}) {} type nullLogFactory struct{} diff --git a/screen_log.go b/screen_log.go index a26e68700..0fcd4ebdb 100644 --- a/screen_log.go +++ b/screen_log.go @@ -9,12 +9,12 @@ type screenLog struct { prefix string } -func (l screenLog) OnIncoming(s string) { +func (l screenLog) OnIncoming(s []byte) { logTime := time.Now().UTC() fmt.Printf("<%v, %s, incoming>\n (%s)\n", logTime, l.prefix, s) } -func (l screenLog) OnOutgoing(s string) { +func (l screenLog) OnOutgoing(s []byte) { logTime := time.Now().UTC() fmt.Printf("<%v, %s, outgoing>\n (%s)\n", logTime, l.prefix, s) } diff --git a/session.go b/session.go index b0192906a..bad69a412 100644 --- a/session.go +++ b/session.go @@ -334,7 +334,7 @@ func (s *session) dropQueued() { } func (s *session) sendBytes(msg []byte) { - s.log.OnOutgoing(string(msg)) + s.log.OnOutgoing(msg) s.messageOut <- msg s.stateTimer.Reset(s.HeartBtInt) } diff --git a/session_state.go b/session_state.go index 7645919aa..cb15335dd 100644 --- a/session_state.go +++ b/session_state.go @@ -67,7 +67,7 @@ func (sm *stateMachine) Incoming(session *session, m fixIn) { return } - session.log.OnIncoming(string(m.bytes)) + session.log.OnIncoming(m.bytes) if msg, err := ParseMessage(m.bytes); err != nil { session.log.OnEventf("Msg Parse Error: %v, %q", err.Error(), m.bytes) } else { From 90a3949928f8bcf6828b5db68d2d41446f36fffa Mon Sep 17 00:00:00 2001 From: Chris Busbey Date: Tue, 27 Sep 2016 11:31:40 -0500 Subject: [PATCH 2/2] Message.build package private, updated signature --- in_session.go | 11 +++------- in_session_test.go | 8 ++++---- message.go | 11 ++++------ message_test.go | 15 ++++++-------- quickfix_test.go | 7 +++---- session.go | 50 +++++++++++++++++++++++----------------------- session_test.go | 4 ++-- validation_test.go | 30 ++++++++++++++-------------- 8 files changed, 62 insertions(+), 74 deletions(-) diff --git a/in_session.go b/in_session.go index b683c00f0..87cbd977f 100644 --- a/in_session.go +++ b/in_session.go @@ -232,10 +232,8 @@ func (state inSession) resendMessages(session *session, beginSeqNo, endSeqNo int } session.log.OnEventf("Resending Message: %v", sentMessageSeqNum) - if _, err := msg.Build(); err != nil { - return err - } - session.sendBytes(msg.rawMessage) + msgBytes = msg.build() + session.sendBytes(msgBytes) seqNum = sentMessageSeqNum + 1 nextSeqNum = seqNum @@ -374,10 +372,7 @@ func (state *inSession) generateSequenceReset(session *session, beginSeqNo int, session.application.ToAdmin(sequenceReset, session.sessionID) - msgBytes, err := sequenceReset.Build() - if err != nil { - return - } + msgBytes := sequenceReset.build() session.sendBytes(msgBytes) session.log.OnEventf("Sent SequenceReset TO: %v", endSeqNo) diff --git a/in_session_test.go b/in_session_test.go index 06c25128a..938f69706 100644 --- a/in_session_test.go +++ b/in_session_test.go @@ -160,8 +160,8 @@ func (s *InSessionTestSuite) TestFIXMsgInTargetTooHigh() { stashedMsg, ok := resendState.messageStash[6] s.True(ok) - rawMsg, _ := msgSeqNumTooHigh.Build() - stashedRawMsg, _ := stashedMsg.Build() + rawMsg := msgSeqNumTooHigh.build() + stashedRawMsg := stashedMsg.build() s.Equal(string(rawMsg), string(stashedRawMsg)) } func (s *InSessionTestSuite) TestFIXMsgInTargetTooHighResendRequestChunkSize() { @@ -198,8 +198,8 @@ func (s *InSessionTestSuite) TestFIXMsgInTargetTooHighResendRequestChunkSize() { stashedMsg, ok := resendState.messageStash[6] s.True(ok) - rawMsg, _ := msgSeqNumTooHigh.Build() - stashedRawMsg, _ := stashedMsg.Build() + rawMsg := msgSeqNumTooHigh.build() + stashedRawMsg := stashedMsg.build() s.Equal(string(rawMsg), string(stashedRawMsg)) } } diff --git a/message.go b/message.go index 7246d4588..873974aff 100644 --- a/message.go +++ b/message.go @@ -283,8 +283,8 @@ func extractField(parsedFieldBytes *TagValue, buffer []byte) (remBytes []byte, e return buffer[(endIndex + 1):], err } -func (m *Message) String() string { - return string(m.rawMessage) +func (m Message) String() string { + return string(m.build()) } func newCheckSum(value int) FIXString { @@ -292,17 +292,14 @@ func newCheckSum(value int) FIXString { } //Build constructs a []byte from a Message instance -func (m *Message) Build() ([]byte, error) { +func (m Message) build() []byte { m.cook() var b bytes.Buffer m.Header.write(&b) m.Body.write(&b) m.Trailer.write(&b) - - m.rawMessage = b.Bytes() - - return m.rawMessage, nil + return b.Bytes() } func (m Message) cook() { diff --git a/message_test.go b/message_test.go index cb5eab44f..4b31e6f56 100644 --- a/message_test.go +++ b/message_test.go @@ -2,8 +2,9 @@ package quickfix import ( "bytes" - "github.com/quickfixgo/quickfix/enum" "testing" + + "github.com/quickfixgo/quickfix/enum" ) var msgResult Message @@ -82,11 +83,7 @@ func TestMessage_Build(t *testing.T) { builder.Body.SetField(Tag(554), FIXString("secret")) expectedBytes := []byte("8=FIX.4.49=4935=A52=20140615-19:49:56553=my_user554=secret10=072") - result, err := builder.Build() - if err != nil { - t.Error("Unexpected error", err) - } - + result := builder.build() if !bytes.Equal(expectedBytes, result) { t.Error("Unexpected bytes, got ", string(result)) } @@ -100,12 +97,12 @@ func TestMessage_ReBuild(t *testing.T) { msg.Header.SetField(tagOrigSendingTime, FIXString("20140515-19:49:56.659")) msg.Header.SetField(tagSendingTime, FIXString("20140615-19:49:56")) - msg.Build() + rebuildBytes := msg.build() expectedBytes := []byte("8=FIX.4.29=12635=D34=249=TW52=20140615-19:49:5656=ISLD122=20140515-19:49:56.65911=10021=140=154=155=TSLA60=00010101-00:00:00.00010=128") - if !bytes.Equal(expectedBytes, msg.rawMessage) { - t.Errorf("Unexpected bytes,\n +%s\n-%s", msg.rawMessage, expectedBytes) + if !bytes.Equal(expectedBytes, rebuildBytes) { + t.Errorf("Unexpected bytes,\n +%s\n-%s", rebuildBytes, expectedBytes) } expectedBodyBytes := []byte("11=10021=140=154=155=TSLA60=00010101-00:00:00.000") diff --git a/quickfix_test.go b/quickfix_test.go index fd3ad73c8..13e78e66f 100644 --- a/quickfix_test.go +++ b/quickfix_test.go @@ -46,10 +46,9 @@ func (s *QuickFIXSuite) FieldEquals(tag Tag, expectedValue interface{}, fieldMap } } -func (s *QuickFIXSuite) MessageEqualsBytes(msgBytes []byte, msg Message) { - _, err := msg.Build() - s.Require().Nil(err) - s.Equal(string(msg.rawMessage), string(msgBytes)) +func (s *QuickFIXSuite) MessageEqualsBytes(expectedBytes []byte, msg Message) { + actualBytes := msg.build() + s.Equal(string(actualBytes), string(expectedBytes)) } //MockStore wraps a memory store and mocks Refresh for convenience diff --git a/session.go b/session.go index bad69a412..23e694808 100644 --- a/session.go +++ b/session.go @@ -21,7 +21,7 @@ type session struct { messageIn <-chan fixIn //application messages are queued up for send here - toSend []Message + toSend [][]byte //mutex for access to toSend sendMutex sync.Mutex @@ -196,11 +196,12 @@ func (s *session) queueForSend(msg Message) error { s.sendMutex.Lock() defer s.sendMutex.Unlock() - if err := s.prepMessageForSend(&msg, nil); err != nil { + msgBytes, err := s.prepMessageForSend(msg, nil) + if err != nil { return err } - s.toSend = append(s.toSend, msg) + s.toSend = append(s.toSend, msgBytes) select { case s.messageEvent <- true: @@ -222,11 +223,12 @@ func (s *session) sendInReplyTo(msg Message, inReplyTo *Message) error { s.sendMutex.Lock() defer s.sendMutex.Unlock() - if err := s.prepMessageForSend(&msg, inReplyTo); err != nil { + msgBytes, err := s.prepMessageForSend(msg, inReplyTo) + if err != nil { return err } - s.toSend = append(s.toSend, msg) + s.toSend = append(s.toSend, msgBytes) s.sendQueued() return nil @@ -255,42 +257,42 @@ func (s *session) dropAndSendInReplyTo(msg Message, resetStore bool, inReplyTo * } } - if err := s.prepMessageForSend(&msg, inReplyTo); err != nil { + msgBytes, err := s.prepMessageForSend(msg, inReplyTo) + if err != nil { return err } s.dropQueued() - s.toSend = append(s.toSend, msg) + s.toSend = append(s.toSend, msgBytes) s.sendQueued() return nil } -func (s *session) prepMessageForSend(msg, inReplyTo *Message) error { - s.fillDefaultHeader(*msg, inReplyTo) +func (s *session) prepMessageForSend(msg Message, inReplyTo *Message) (msgBytes []byte, err error) { + s.fillDefaultHeader(msg, inReplyTo) seqNum := s.store.NextSenderMsgSeqNum() msg.Header.SetField(tagMsgSeqNum, FIXInt(seqNum)) - var err error msgType, err := msg.MsgType() if err != nil { - return err + return } if isAdminMessageType(string(msgType)) { - s.application.ToAdmin(*msg, s.sessionID) + s.application.ToAdmin(msg, s.sessionID) if msgType == enum.MsgType_LOGON { var resetSeqNumFlag FIXBoolean if msg.Body.Has(tagResetSeqNumFlag) { - if err := msg.Body.GetField(tagResetSeqNumFlag, &resetSeqNumFlag); err != nil { - return err + if err = msg.Body.GetField(tagResetSeqNumFlag, &resetSeqNumFlag); err != nil { + return } } if resetSeqNumFlag.Bool() { - if err := s.store.Reset(); err != nil { - return err + if err = s.store.Reset(); err != nil { + return } s.sentReset = true @@ -300,17 +302,15 @@ func (s *session) prepMessageForSend(msg, inReplyTo *Message) error { } } else { - if err := s.application.ToApp(*msg, s.sessionID); err != nil { - return err + if err = s.application.ToApp(msg, s.sessionID); err != nil { + return } } - msgBytes, err := msg.Build() - if err == nil { - err = s.persist(seqNum, msgBytes) - } + msgBytes = msg.build() + err = s.persist(seqNum, msgBytes) - return err + return } func (s *session) persist(seqNum int, msgBytes []byte) error { @@ -322,8 +322,8 @@ func (s *session) persist(seqNum int, msgBytes []byte) error { } func (s *session) sendQueued() { - for _, msg := range s.toSend { - s.sendBytes(msg.rawMessage) + for _, msgBytes := range s.toSend { + s.sendBytes(msgBytes) } s.dropQueued() diff --git a/session_test.go b/session_test.go index e2db97769..5940602dc 100644 --- a/session_test.go +++ b/session_test.go @@ -410,9 +410,9 @@ func (s *SessionSuite) TestIncomingNotInSessionTime() { } msg := s.NewOrderSingle() - msg.Build() + msgBytes := msg.build() - s.session.Incoming(s.session, fixIn{bytes: msg.rawMessage}) + s.session.Incoming(s.session, fixIn{bytes: msgBytes}) s.MockApp.AssertExpectations(s.T()) s.State(notSessionTime{}) } diff --git a/validation_test.go b/validation_test.go index 087a54b95..f488efd42 100644 --- a/validation_test.go +++ b/validation_test.go @@ -125,7 +125,7 @@ func tcInvalidTagNumberHeader() validateTest { invalidHeaderFieldMessage := createFIX40NewOrderSingle() tag := Tag(9999) invalidHeaderFieldMessage.Header.SetField(tag, FIXString("hello")) - msgBytes, _ := invalidHeaderFieldMessage.Build() + msgBytes := invalidHeaderFieldMessage.build() return validateTest{ TestName: "Invalid Tag Number Header", @@ -141,7 +141,7 @@ func tcInvalidTagNumberBody() validateTest { invalidBodyFieldMessage := createFIX40NewOrderSingle() tag := Tag(9999) invalidBodyFieldMessage.Body.SetField(tag, FIXString("hello")) - msgBytes, _ := invalidBodyFieldMessage.Build() + msgBytes := invalidBodyFieldMessage.build() return validateTest{ TestName: "Invalid Tag Number Body", @@ -158,7 +158,7 @@ func tcInvalidTagNumberTrailer() validateTest { invalidTrailerFieldMessage := createFIX40NewOrderSingle() tag := Tag(9999) invalidTrailerFieldMessage.Trailer.SetField(tag, FIXString("hello")) - msgBytes, _ := invalidTrailerFieldMessage.Build() + msgBytes := invalidTrailerFieldMessage.build() return validateTest{ TestName: "Invalid Tag Number Trailer", @@ -175,7 +175,7 @@ func tcTagNotDefinedForMessage() validateTest { invalidMsg := createFIX40NewOrderSingle() tag := Tag(41) invalidMsg.Body.SetField(tag, FIXString("hello")) - msgBytes, _ := invalidMsg.Build() + msgBytes := invalidMsg.build() return validateTest{ TestName: "Tag Not Defined For Message", @@ -191,7 +191,7 @@ func tcTagIsDefinedForMessage() validateTest { dict, _ := datadictionary.Parse("spec/FIX43.xml") validator := &fixValidator{dict, defaultValidatorSettings} validMsg := createFIX43NewOrderSingle() - msgBytes, _ := validMsg.Build() + msgBytes := validMsg.build() return validateTest{ TestName: "TagIsDefinedForMessage", @@ -224,7 +224,7 @@ func tcFieldNotFoundBody() validateTest { //ord type is required //invalidMsg1.Body.SetField(Tag(40), "A")) - msgBytes, _ := invalidMsg1.Build() + msgBytes := invalidMsg1.build() return validateTest{ TestName: "FieldNotFoundBody", @@ -257,7 +257,7 @@ func tcFieldNotFoundHeader() validateTest { //invalidMsg2.Header.FieldMap.SetField(tag.SendingTime, "0")) tag := tagSendingTime - msgBytes, _ := invalidMsg2.Build() + msgBytes := invalidMsg2.build() return validateTest{ TestName: "FieldNotFoundHeader", @@ -275,7 +275,7 @@ func tcTagSpecifiedWithoutAValue() validateTest { bogusTag := Tag(109) builder.Body.SetField(bogusTag, FIXString("")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() return validateTest{ TestName: "Tag SpecifiedWithoutAValue", @@ -291,7 +291,7 @@ func tcInvalidMsgType() validateTest { validator := &fixValidator{dict, defaultValidatorSettings} builder := createFIX40NewOrderSingle() builder.Header.SetField(tagMsgType, FIXString("z")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() return validateTest{ TestName: "Invalid MsgType", @@ -308,7 +308,7 @@ func tcValueIsIncorrect() validateTest { tag := Tag(21) builder := createFIX40NewOrderSingle() builder.Body.SetField(tag, FIXString("4")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() return validateTest{ TestName: "ValueIsIncorrect", @@ -325,7 +325,7 @@ func tcIncorrectDataFormatForValue() validateTest { builder := createFIX40NewOrderSingle() tag := Tag(38) builder.Body.SetField(tag, FIXString("+200.00")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() return validateTest{ TestName: "IncorrectDataFormatForValue", @@ -344,7 +344,7 @@ func tcTagSpecifiedOutOfRequiredOrderHeader() validateTest { tag := tagOnBehalfOfCompID //should be in header builder.Body.SetField(tag, FIXString("CWB")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() return validateTest{ TestName: "Tag specified out of required order in Header", @@ -363,7 +363,7 @@ func tcTagSpecifiedOutOfRequiredOrderTrailer() validateTest { tag := tagSignature //should be in trailer builder.Body.SetField(tag, FIXString("SIG")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() refTag := Tag(100) return validateTest{ @@ -384,7 +384,7 @@ func tcTagSpecifiedOutOfRequiredOrderDisabledHeader() validateTest { tag := tagOnBehalfOfCompID //should be in header builder.Body.SetField(tag, FIXString("CWB")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() return validateTest{ TestName: "Tag specified out of required order in Header - Disabled", @@ -403,7 +403,7 @@ func tcTagSpecifiedOutOfRequiredOrderDisabledTrailer() validateTest { tag := tagSignature //should be in trailer builder.Body.SetField(tag, FIXString("SIG")) - msgBytes, _ := builder.Build() + msgBytes := builder.build() return validateTest{ TestName: "Tag specified out of required order in Trailer - Disabled",