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

test: Updated benchmark test results to output result files #2350

Merged
merged 12 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ test/versioned-external/TEMP_TESTS
nr-security-home
# Needed for testing
!test/integration/moduleLoading/node_modules
# benchmark results
benchmark_*.json
mrickard marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 12 additions & 3 deletions bin/compare-bench-results.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ const reportResults = (resultFiles) => {
const results = baseTests
.sort()
.map((test) => {
if (!base[test] || !down[test]) {
console.error(`**** comparison tests are not matched for ${test}!`)
console.log('base case test', base)
console.log('downstream test', down)
return ''
}

const passes = compareResults(base[test], down[test])
filePassing = filePassing && passes

Expand Down Expand Up @@ -118,9 +125,11 @@ const reportResults = (resultFiles) => {

const iterate = async () => {
const files = process.argv.slice(2)
const results = files.map(async (file) => {
return processFile(file)
})
const results = await Promise.all(
files.map(async (file) => {
return await processFile(file)
})
)
reportResults(results)
}

Expand Down
131 changes: 65 additions & 66 deletions bin/run-bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,20 @@

'use strict'

/* eslint sonarjs/cognitive-complexity: ["error", 21] -- TODO: https://issues.newrelic.com/browse/NEWRELIC-5252 */

const cp = require('child_process')
const glob = require('glob')
const path = require('path')
const { errorAndExit } = require('./utils')
const fs = require('fs/promises')

const cwd = path.resolve(__dirname, '..')
const benchpath = path.resolve(cwd, 'test/benchmark')

const tests = []
const testPromises = []
const globs = []
const opts = Object.create(null)

// replacement for former async-lib cb
const testCb = (err, payload) => {
if (err) {
console.error(err)
return
}
return payload
}

process.argv.slice(2).forEach(function forEachFileArg(file) {
if (/^--/.test(file)) {
opts[file.substring(2)] = true
Expand All @@ -48,35 +39,37 @@ if (tests.length === 0 && globs.length === 0) {
globs.push(path.join(benchpath, '*.bench.js'), path.join(benchpath, '**/*.bench.js'))
}

class ConsolePrinter {
/* eslint-disable no-console */
addTest(name, child) {
console.log(name)
child.stdout.on('data', (d) => process.stdout.write(d))
child.stderr.on('data', (d) => process.stderr.write(d))
child.once('exit', () => console.log(''))
}

finish() {
console.log('')
}
/* eslint-enable no-console */
}

class JSONPrinter {
class Printer {
constructor() {
this._tests = Object.create(null)
}

addTest(name, child) {
let output = ''
this._tests[name] = null
child.stdout.on('data', (d) => (output += d.toString()))
child.stdout.on('end', () => (this._tests[name] = JSON.parse(output)))
child.stderr.on('data', (d) => process.stderr.write(d))

this._tests[name] = new Promise((resolve) => {
child.stdout.on('end', () => {
try {
this._tests[name] = JSON.parse(output)
} catch (e) {
console.error(`Error parsing test results for ${name}`, e)
this._tests[name] = output
}
resolve()
})
})
}

finish() {
async finish() {
if (opts.json) {
const content = JSON.stringify(this._tests, null, 2)
const fileName = `benchmark_${new Date().getTime()}.json`
jsumners-nr marked this conversation as resolved.
Show resolved Hide resolved
await fs.writeFile(fileName, content)
console.log(`Done! Test output written to ${fileName}`)
return
}
/* eslint-disable no-console */
console.log(JSON.stringify(this._tests, null, 2))
/* eslint-enable no-console */
Expand All @@ -86,67 +79,73 @@ class JSONPrinter {
run()

async function run() {
const printer = opts.json ? new JSONPrinter() : new ConsolePrinter()
const printer = new Printer()
let currentTest = 0

const resolveGlobs = (cb) => {
const resolveGlobs = () => {
if (!globs.length) {
cb()
console.error(`There aren't any globs to resolve.`)
return
}
const afterGlobbing = (err, resolved) => {
if (err) {
errorAndExit(err, 'Failed to glob', -1)
cb(err)
const afterGlobbing = (resolved) => {
if (!resolved) {
return errorAndExit(new Error('Failed to glob'), 'Failed to glob', -1)
}
resolved.forEach(function mergeResolved(files) {
files.forEach(function mergeFile(file) {
if (tests.indexOf(file) === -1) {
tests.push(file)
}
})
})
cb() // ambient scope

function mergeFile(file) {
if (tests.indexOf(file) === -1) {
tests.push(file)
}
}
function mergeResolved(files) {
files.forEach(mergeFile)
}

return resolved.forEach(mergeResolved)
}

const globbed = globs.map((item) => glob.sync(item))
return afterGlobbing(null, globbed)
return afterGlobbing(globbed)
}

const spawnEachFile = (file, spawnCb) => {
const spawnEachFile = async (file) => {
const test = path.relative(benchpath, file)

const args = [file]
if (opts.inspect) {
args.unshift('--inspect-brk')
}

const child = cp.spawn('node', args, { cwd: cwd, stdio: 'pipe' })
printer.addTest(test, child)
const child = cp.spawn('node', args, { cwd: cwd, stdio: 'pipe', silent: true })

child.on('error', spawnCb)
child.on('error', (err) => {
console.error(`Error in child test ${test}`, err)
throw err
})
child.on('exit', function onChildExit(code) {
currentTest = currentTest + 1
if (code) {
spawnCb(new Error('Benchmark exited with code ' + code))
console.error(`(${currentTest}/${tests.length}) FAILED: ${test} exited with code ${code}`)
return
}
spawnCb()
console.log(`(${currentTest}/${tests.length}) ${file} has completed`)
})
printer.addTest(test, child)
}

const afterSpawnEachFile = (err, cb) => {
if (err) {
errorAndExit(err, 'Spawning failed:', -2)
return cb(err)
}
cb()
}

const runBenchmarks = async (cb) => {
const runBenchmarks = async () => {
tests.sort()
await tests.forEach((file) => spawnEachFile(file, testCb))
await afterSpawnEachFile(null, testCb)
return cb()
for await (const file of tests) {
await spawnEachFile(file)
}
const keys = Object.keys(printer._tests)
for (const key of keys) {
testPromises.push(printer._tests[key])
}
}

await resolveGlobs(testCb)
await runBenchmarks(testCb)
await resolveGlobs()
await runBenchmarks()
await Promise.all(testPromises)
printer.finish()
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@
},
"scripts": {
"bench": "node ./bin/run-bench.js",
"benchJson": "node ./bin/run-bench.js --json",
mrickard marked this conversation as resolved.
Show resolved Hide resolved
"docker-env": "./bin/docker-env-vars.sh",
"docs": "rm -rf ./out && jsdoc -c ./jsdoc-conf.jsonc --private -r .",
"integration": "npm run prepare-test && npm run sub-install && time c8 -o ./coverage/integration tap --test-regex='(\\/|^test\\/integration\\/.*\\.tap\\.js)$' --timeout=600 --no-coverage --reporter classic",
Expand Down
6 changes: 3 additions & 3 deletions test/benchmark/async-hooks.bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const benchmark = require('../lib/benchmark')

const suite = benchmark.createBenchmark({
name: 'async hooks',
async: true,
fn: runBenchmark
})

Expand Down Expand Up @@ -52,10 +51,11 @@ tests.forEach((test) => suite.add(test))

suite.run()

function runBenchmark(agent, cb) {
function runBenchmark() {
let p = Promise.resolve()
for (let i = 0; i < 300; ++i) {
p = p.then(function noop() {})
}
p.then(cb)

return p
}
21 changes: 12 additions & 9 deletions test/benchmark/datastore-shim/recordBatch.bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,34 @@ function makeInit(instrumented) {

suite.add({
name: 'instrumented operation',
async: true,
initialize: makeInit(true),
agent: {},
fn: function (agent, done) {
testDatastore.testBatch('test', done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testBatch('test', resolve)
})
}
})

suite.add({
name: 'uninstrumented operation',
initialize: makeInit(false),
async: true,
fn: function (agent, done) {
testDatastore.testBatch('test', done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testBatch('test', resolve)
})
}
})

suite.add({
name: 'instrumented operation in transaction',
async: true,
agent: {},
initialize: makeInit(true),
runInTransaction: true,
fn: function (agent, done) {
testDatastore.testBatch('test', done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testBatch('test', resolve)
})
}
})

Expand Down
21 changes: 12 additions & 9 deletions test/benchmark/datastore-shim/recordOperation.bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,34 @@ function makeInit(instrumented) {

suite.add({
name: 'instrumented operation in transaction',
async: true,
agent: {},
initialize: makeInit(true),
runInTransaction: true,
fn: function (agent, done) {
testDatastore.testOp(done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testOp(resolve)
})
}
})

suite.add({
name: 'instrumented operation',
async: true,
initialize: makeInit(true),
agent: {},
fn: function (agent, done) {
testDatastore.testOp(done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testOp(resolve)
})
}
})

suite.add({
name: 'uninstrumented operation',
initialize: makeInit(false),
async: true,
fn: function (agent, done) {
testDatastore.testOp(done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testOp(resolve)
})
}
})

Expand Down
21 changes: 12 additions & 9 deletions test/benchmark/datastore-shim/recordQuery.bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,34 @@ function makeInit(instrumented) {

suite.add({
name: 'instrumented operation in transaction',
async: true,
agent: {},
initialize: makeInit(true),
runInTransaction: true,
fn: function (agent, done) {
testDatastore.testQuery('test', done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testQuery('test', resolve)
})
}
})

suite.add({
name: 'instrumented operation',
async: true,
initialize: makeInit(true),
agent: {},
fn: function (agent, done) {
testDatastore.testQuery('test', done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testQuery('test', resolve)
})
}
})

suite.add({
name: 'uninstrumented operation',
initialize: makeInit(false),
async: true,
fn: function (agent, done) {
testDatastore.testQuery('test', done)
fn: function () {
return new Promise((resolve) => {
testDatastore.testQuery('test', resolve)
})
}
})

Expand Down
Loading
Loading