Skip to content

Conversation

@ryoqun
Copy link
Contributor

@ryoqun ryoqun commented May 7, 2016

Hopefully, the title says all.

Well, we're using a lot of namespaces. and there is performance problem.

@ryoqun ryoqun changed the title Return early when channels mismatches to skip expensive msgpack decoding Return early when channels mismatch to skip expensive msgpack decoding May 7, 2016
@darrachequesne
Copy link
Member

LGTM 👍

Could you squash it into one commit please?

@ryoqun ryoqun force-pushed the avoid-msgpack-decoding branch from 658b0d4 to 5bcb29d Compare May 8, 2016 11:30
@ryoqun
Copy link
Contributor Author

ryoqun commented May 8, 2016

@darrachequesne squashed! Thanks for quick reply!

(hopefully, version bump is preferred... :p)

@ryoqun
Copy link
Contributor Author

ryoqun commented May 9, 2016

@nkzawa @rauchg (Kindly pinging to just remind this project's members; sorry for being impatient! ;) )

} else { // Fallback to slow indexOf impl for older Node.js
this.channelMatches = function (messageChannel, subscribedChannel) {
return messageChannel.indexOf(subscribedChannel) === 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just using substr ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rauchg Fixed! f2e7a4c

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rauchg I referred to https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith

I leant something from your suggestion, thank you!

@rauchg rauchg merged commit e2da797 into socketio:master May 9, 2016
@rauchg
Copy link
Contributor

rauchg commented May 9, 2016

Thanks!

@ryoqun
Copy link
Contributor Author

ryoqun commented May 9, 2016

@rauchg Thanks a lot! Also, could you tell me when a new version will be released? :)

@ryoqun
Copy link
Contributor Author

ryoqun commented May 30, 2016

@darrachequesne @nkzawa @rauchg sorry for indiscriminate pinging... Some time has passed. Hopefully, I rather want this modification to be officially released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants