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

lib: refactor tls options handle in https.js #27118

Closed
wants to merge 1 commit into from

Conversation

gengjiawen
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the https Issues or PRs related to the https subsystem. label Apr 7, 2019
@gengjiawen gengjiawen force-pushed the rafactor_header_handle branch from 52841ee to 0057d90 Compare April 7, 2019 09:26
@lpinca
Copy link
Member

lpinca commented Apr 7, 2019

@gengjiawen a for loop was not used for performance reasons. See discussion in #16402.

@gengjiawen
Copy link
Member Author

gengjiawen commented Apr 8, 2019

@gengjiawen a for loop was not used for performance reasons. See discussion in #16402.

Looks like 2x diff (Node node: '12.0.0-pre', v8: '7.3.492.25-node.7')

es\arrary-vs-invidual.js mode="Array" n=1000: 263,026.3123632099
es\arrary-vs-invidual.js mode="Invidual" n=1000: 469,417.45294090034
'use strict';

const common = require('../common.js');
const configs = {
  n: [1e3],
  mode: ['Array', 'Invidual']
};

const bench = common.createBenchmark(main, configs);

function main({ n, mode }) {
  switch (mode) {
    case '':
    // Empty string falls through to next line as default, mostly for tests.
    case 'Array':
      bench.start();
      for (let i = 0; i < n; i++)
        useArray();
      bench.end(n);
      break;
    case 'Invidual':
      bench.start();
      for (let i = 0; i < n; i++)
        useInvidual();
      bench.end(n);
      break;
    default:
      throw new Error(`Unexpected method "${mode}"`);
  }
}

function useArray() {
  let name = '';
  let options = {};
  const options_keys = [
    'ca',
    'cert',
    'ciphers',
    'clientCertEngine',
    'crl',
    'dhparam',
    'ecdhCurve',
    'honorCipherOrder',
    'key',
    'maxVersion',
    'minVersion',
    'pfx',
    'rejectUnauthorized',
    'secureOptions',
    'secureProtocol',
    'sessionIdContext'
  ];

  options_keys.forEach((i) => {
    name += ':';
    if (options[i]) {
      name += options[i];
    }
  });

  name += ':';
  if (options.servername && options.servername !== options.host)
    name += options.servername;

  return name;
}

function useInvidual() {
  let name = '';
  let options = {};

  name += ':';
  if (options.ca)
    name += options.ca;

  name += ':';
  if (options.cert)
    name += options.cert;

  name += ':';
  if (options.clientCertEngine)
    name += options.clientCertEngine;

  name += ':';
  if (options.ciphers)
    name += options.ciphers;

  name += ':';
  if (options.key)
    name += options.key;

  name += ':';
  if (options.pfx)
    name += options.pfx;

  name += ':';
  if (options.rejectUnauthorized !== undefined)
    name += options.rejectUnauthorized;

  name += ':';
  if (options.servername && options.servername !== options.host)
    name += options.servername;

  name += ':';
  if (options.minVersion)
    name += options.minVersion;

  name += ':';
  if (options.maxVersion)
    name += options.maxVersion;

  name += ':';
  if (options.secureProtocol)
    name += options.secureProtocol;

  name += ':';
  if (options.crl)
    name += options.crl;

  name += ':';
  if (options.honorCipherOrder !== undefined)
    name += options.honorCipherOrder;

  name += ':';
  if (options.ecdhCurve)
    name += options.ecdhCurve;

  name += ':';
  if (options.dhparam)
    name += options.dhparam;

  name += ':';
  if (options.secureOptions !== undefined)
    name += options.secureOptions;

  name += ':';
  if (options.sessionIdContext)
    name += options.sessionIdContext;

  return name;
}

@gengjiawen gengjiawen closed this Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
https Issues or PRs related to the https subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants