Skip to content

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

Closed
gengjiawen wants to merge 1 commit into
nodejs:masterfrom
gengjiawen:rafactor_header_handle
Closed

lib: refactor tls options handle in https.js#27118
gengjiawen wants to merge 1 commit into
nodejs:masterfrom
gengjiawen:rafactor_header_handle

Conversation

@gengjiawen

Copy link
Copy Markdown
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

lpinca commented Apr 7, 2019

Copy link
Copy Markdown
Member

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

@gengjiawen

gengjiawen commented Apr 8, 2019

Copy link
Copy Markdown
Member Author

@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
Details
'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