Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[perf] websocket optimisation #453

Merged
merged 5 commits into from
Dec 5, 2016

Conversation

billouboq
Copy link
Contributor

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

use forEach loop and anonymous functions

New behaviour

use for loop and cached function

Other information (e.g. related issues)

Why it's better to use cached functions: see this

parser.encodePacket(packet, self.supportsBinary, parserCb);
}

function parserCb (data) {
Copy link
Member

Choose a reason for hiding this comment

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

How about renaming that method send, to better reflect its purpose?

});
});
});
function sendCb (err) {
Copy link
Member

Choose a reason for hiding this comment

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

Same hère, how about something like onEnd?

@darrachequesne
Copy link
Member

Hi, thanks for that PR! I made some minor comments, could you please check them?

Did you see any other area of improvement?

@billouboq
Copy link
Contributor Author

Will check tonight and change what you suggested

billouboq and others added 3 commits December 5, 2016 22:20
@darrachequesne darrachequesne merged commit 01c6a12 into socketio:master Dec 5, 2016
@darrachequesne
Copy link
Member

Thanks!

@darrachequesne darrachequesne added this to the 1.8.2 milestone Dec 10, 2016
darrachequesne pushed a commit that referenced this pull request May 8, 2020
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.

2 participants