Skip to content

Smart pointers in TextBuffer #2

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

Draft
wants to merge 50 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
d1b3f03
Add clang-tidy
aminya Dec 3, 2020
c445df8
Add clang-format script
aminya Dec 3, 2020
ef7a5ba
Add standard variable + use c++14
aminya Dec 3, 2020
a8296f1
Add MACOSX_DEPLOYMENT_TARGET variable
aminya Dec 3, 2020
d3f3d40
Add Release configurations and optimizations
aminya Dec 3, 2020
490e785
Use C++17 standard
aminya Dec 3, 2020
18aac65
Use Visual Studio 17 on AppVeyor
aminya Dec 3, 2020
eff9d3d
Merge branch 'Release-optimizations'
aminya Dec 3, 2020
ea704b2
use native optional
aminya Dec 3, 2020
7a73403
Merge branch 'native-optional'
aminya Dec 3, 2020
c787367
use constexpr instead of static const
aminya Dec 3, 2020
0603004
Merge branch 'constexpr'
aminya Dec 3, 2020
09d34bd
Exclude reserved identifiers from clang-tidy
aminya Dec 3, 2020
3660a91
Merge branch 'clang-tidy'
aminya Dec 3, 2020
c553008
remove optional.h and include optional directly
aminya Dec 3, 2020
9229018
modernize-deprecated-headers
aminya Dec 3, 2020
d4479ec
use angle brackets for external headers
aminya Dec 3, 2020
0b9f4c5
Make constructors explicit
aminya Dec 3, 2020
c7cee01
Make single-argument constructors explicit
aminya Dec 3, 2020
26364d6
Make multo-argument constructors explicit
aminya Dec 3, 2020
e9ce958
Merge branch 'explicit-constructors'
aminya Dec 3, 2020
851b65c
Make most of the constructors explicit
aminya Dec 3, 2020
f98a830
Add TODO for making TextSlice explicit
aminya Dec 3, 2020
5e64ae4
Add TODO for making Text explicit
aminya Dec 3, 2020
ee74336
Merge branch 'explicit-constructors'
aminya Dec 3, 2020
d73adf7
Use Clang 10 on Travis
aminya Dec 3, 2020
2529ee2
Merge branch 'Release-optimizations'
aminya Dec 3, 2020
d50be88
explicit comparison for ferror to prevent conversion
aminya Dec 3, 2020
349bdd8
explicit comparison for int/uint numbers
aminya Dec 3, 2020
d661ac9
explicit comparison for objects that may be null
aminya Dec 3, 2020
81f7eba
Merge remote-tracking branch 'upstream/master'
aminya Dec 8, 2020
f4602c9
install download instead of unzip package
aminya Dec 8, 2020
8d8f323
fix getText by using download package
aminya Dec 8, 2020
592a10d
run the benchmark for the full size
aminya Dec 8, 2020
38a7089
add large-text benchmark to npm benchmark script
aminya Dec 8, 2020
f4eb0a5
use performance from perf_hooks to measure time
aminya Dec 8, 2020
6cbc912
remove 10b test (Very small to find anything meaningful)
aminya Dec 8, 2020
79d838c
run the tests for different words
aminya Dec 8, 2020
9334888
pretty print the result
aminya Dec 8, 2020
3ab437d
make test runner async + run same size tests together
aminya Dec 8, 2020
1548676
make test function async
aminya Dec 8, 2020
928fa1c
benchmark buffer.setText
aminya Dec 8, 2020
190108d
Merge branch 'fix-large-text-benchmark'
aminya Dec 8, 2020
cdd5de2
fix text-buffer benchmark
aminya Dec 8, 2020
f5792c7
use performance in text-buffer benchmarks + prettry print
aminya Dec 8, 2020
0e9523a
use performance in marker-index benchmarks + pretty print
aminya Dec 8, 2020
13ef1e3
Merge branch 'fix-large-text-benchmark'
aminya Dec 8, 2020
92e40de
First make everything a shared_ptr
aminya Dec 31, 2020
0698755
Patch missing methods
aminya Jan 8, 2021
f18a33b
text-buffer-wrapper
aminya Jan 8, 2021
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
4 changes: 4 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
Checks: "*, -clang-diagnostic-*-compat, -cppcoreguidelines-init-variables, -modernize-return-braced-init-list, -misc-unused-parameters, -misc-non-private-member-variables-in-classes, -llvmlibc-*, -llvm-header-guard, -llvm-include-order, -modernize-use-trailing-return-type, -readability-avoid-const-params-in-decls, -readability-convert-member-functions-to-static, -fuchsia-default-arguments-declarations, -fuchsia-default-arguments-calls, -*-uppercase-literal-suffix, -fuchsia-overloaded-operator, -google-build-using-namespace, -google-global-names-in-headers, -google-readability-todo, -*-else-after-return, -*-braces-around-statements, -bugprone-reserved-identifier, -cert-dcl37-c, -cert-dcl51-cpp"
HeaderFilterRegex: ".*"
FormatStyle: none
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ build
/browser.js
emsdk-portable
package-lock.json

benchmark/*.csv
6 changes: 3 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ node_js:
- "12.14.1"

before_install:
- export CXX="g++-4.9" CC="gcc-4.9"
- export CXX="clang-10.0" CC="clang++-10.0"

script:
- npm run standard
Expand All @@ -31,5 +31,5 @@ addons:
sources:
- ubuntu-toolchain-r-test
packages:
- gcc-4.9
- g++-4.9
- clang-10.0
- clang++-10.0
2 changes: 1 addition & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
image: Visual Studio 2015
image: Visual Studio 2017

environment:
matrix:
Expand Down
85 changes: 41 additions & 44 deletions benchmark/large-text-buffer.benchmark.js
Original file line number Diff line number Diff line change
@@ -1,56 +1,53 @@
const http = require('http')
const fs = require('fs')
const unzip = require('unzip')
const { TextBuffer } = require('..')

const unzipper = unzip.Parse()

const getText = () => {
return new Promise(resolve => {
console.log('fetching text file...')
const req = http.get({
hostname: 'www.acleddata.com',
port: 80,
// 51 MB text file
path: '/wp-content/uploads/2017/01/ACLED-Version-7-All-Africa-1997-2016_csv_dyadic-file.zip',
agent: false
}, res => {
res
.pipe(unzipper)
.on('entry', entry => {
let data = '';
entry.on('data', chunk => data += chunk);
entry.on('end', () => {
resolve(data)
});
})
})

req.end()
})
const fs = require('fs')
const {promisify} = require('util')
const readFile = promisify(fs.readFile)
const path = require('path')
const download = require('download')
const {performance} = require("perf_hooks")

async function getText() {
const filePath = path.join(__dirname, '1000000 Sales Records.csv')
if (!fs.existsSync(filePath)) {
// 122MB file
await download(
'http://eforexcel.com/wp/wp-content/uploads/2017/07/1000000%20Sales%20Records.zip',
__dirname,
{extract: true}
)
}
return await readFile(filePath)
}

const timer = size => `Time to find "cat" in ${size} file`

getText().then(txt => {
getText().then(async (txt) => {
const buffer = new TextBuffer()

console.log('running findWordsWithSubsequence tests...')
console.log('\n running large-text-buffer tests... \n')

const sizes = [['10b', 10], ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000]]
const sizes = [ ['100b', 100], ['1kb', 1000], ['1MB', 1000000], ['51MB', 100000000], ['119MB', txt.length]]

const test = size => {
const _timer = timer(size[0])
buffer.setText(txt.slice(0, size[1]))
console.time(_timer)
return buffer.findWordsWithSubsequence('cat', '', 100).then(sugs => {
console.timeEnd(_timer)
})
const test = async (word, size) => {
const ti2 = performance.now()
await buffer.findWordsWithSubsequence(word, '', 100)
const tf2 = performance.now()
console.log(`For ${size[0]} file, time to find "${word}" was: ${' '.repeat(50-word.length-size[0].length)} ${(tf2-ti2).toFixed(5)} ms`)
}
for (const size of sizes) {

return sizes.reduce((promise, size) => {
return promise.then(() => test(size))
}, Promise.resolve())
const bufferText = txt.slice(0, size[1])

// benchmark buffer.setText
const ti1 = performance.now()
buffer.setText(bufferText)
const tf1 = performance.now()
console.log(`For ${size[0]} file, buffer.setText took ${' '.repeat(51-size[0].length)} ${(tf1-ti1).toFixed(5)} ms`)

for (const word of ["Morocco", "Austria", "France", "Liechtenstein", "Republic of the Congo", "Antigua and Barbuda", "Japan"]) {
await test(word, size)
}
console.log('\n')
}
}).then(() => {
console.log('finished')
console.log(' large-text-buffer finished \n')
})
11 changes: 9 additions & 2 deletions benchmark/marker-index.benchmark.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
'use strict';

console.log(' running marker-index tests... \n')

const Random = require('random-seed')
const {performance} = require("perf_hooks")

const {MarkerIndex} = require('..')
const {traverse, traversalDistance, compare} = require('../test/js/helpers/point-helpers')

Expand Down Expand Up @@ -41,12 +45,13 @@ function runBenchmark () {
}

function profileOperations (name, operations) {
console.time(name)
const ti1 = performance.now()
for (let i = 0, n = operations.length; i < n; i++) {
const operation = operations[i]
markerIndex[operation[0]].apply(markerIndex, operation[1])
}
console.timeEnd(name)
const tf1 = performance.now()
console.log(`${name} ${' '.repeat(80-name.length)} ${(tf1-ti1).toFixed(3)} ms`)
}

function enqueueSequentialInsert () {
Expand Down Expand Up @@ -118,3 +123,5 @@ function getSplice () {
}

runBenchmark()

console.log(' \n marker-index finished \n')
22 changes: 15 additions & 7 deletions benchmark/text-buffer.benchmark.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
console.log(' running text-buffer tests... \n')

const assert = require('assert')
const {performance} = require("perf_hooks")

const {TextBuffer} = require('..')

const text = 'abc def ghi jkl\n'.repeat(1024 * 1024)
Expand All @@ -8,14 +12,15 @@ const trialCount = 10

function benchmarkSearch(description, pattern, expectedPosition) {
let name = `Search for ${description} - TextBuffer`
console.time(name)
const ti1 = performance.now()
for (let i = 0; i < trialCount; i++) {
assert.deepEqual(buffer.searchSync(pattern), expectedPosition)
assert.deepEqual(buffer.findSync(pattern), expectedPosition)
}
console.timeEnd(name)
const tf1 = performance.now()
console.log(`${name} ${' '.repeat(80-name.length)} ${(tf1-ti1).toFixed(3)} ms`)

name = `Search for ${description} - lines array`
console.time(name)
const ti2 = performance.now()
const regex = new RegExp(pattern)
for (let i = 0; i < trialCount; i++) {
for (let row = 0, rowCount = lines.length; row < rowCount; row++) {
Expand All @@ -32,11 +37,14 @@ function benchmarkSearch(description, pattern, expectedPosition) {
}
}
}
console.timeEnd(name)
console.log()
const tf2 = performance.now()
console.log(`${name} ${' '.repeat(80-name.length)} ${(tf2-ti2).toFixed(3)} ms`)
}

benchmarkSearch('simple non-existent pattern', '\t', null)
benchmarkSearch('complex non-existent pattern', '123|456|789', null)
benchmarkSearch('simple existing pattern', 'jkl', {start: {row: 0, column: 12}, end: {row: 0, column: 15}})
benchmarkSearch('complex existing pattern', 'j\\w+', {start: {row: 0, column: 12}, end: {row: 0, column: 15}})
benchmarkSearch('complex existing pattern', 'j\\w+', {start: {row: 0, column: 12}, end: {row: 0, column: 15}})


console.log('\n text-buffer finished \n')
49 changes: 42 additions & 7 deletions binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@
],

"variables": {
"tests": 0
"tests": 0,
"STANDARD": 17,
"MACOSX_DEPLOYMENT_TARGET": "10.8"
},

"conditions": [
Expand Down Expand Up @@ -100,20 +102,19 @@
"conditions": [
['OS=="mac"', {
'cflags': [
'-mmacosx-version-min=10.8'
"-mmacosx-version-min=<(MACOSX_DEPLOYMENT_TARGET)"
],
"xcode_settings": {
"GCC_ENABLE_CPP_EXCEPTIONS": "YES",
'MACOSX_DEPLOYMENT_TARGET': '10.8',
'MACOSX_DEPLOYMENT_TARGET': '<(MACOSX_DEPLOYMENT_TARGET)',
}
}]
]
}]
}]
],

"target_defaults": {
"cflags_cc": ["-std=c++11"],
"cflags_cc": [ "-std=c++<(STANDARD)" ],
"conditions": [
['OS=="mac"', {
"xcode_settings": {
Expand All @@ -129,6 +130,40 @@
"NOMINMAX"
],
}]
]
}
],
'default_configuration': 'Release',
'configurations': {
# Release Settings
'Release': {
'defines': [ 'NDEBUG' ],
"cflags": [ "-fno-exceptions", "-Ofast" ],
"cflags_cc": [ "-fno-exceptions", "-Ofast", "-std=c++<(STANDARD)" ],
"xcode_settings": {
'GCC_OPTIMIZATION_LEVEL': '3', # stop gyp from defaulting to -Os
"CLANG_CXX_LIBRARY": "libc++",
"CLANG_CXX_LANGUAGE_STANDARD": "c++<(STANDARD)",
'MACOSX_DEPLOYMENT_TARGET': "<(MACOSX_DEPLOYMENT_TARGET)"
}, # XCODE
"msvs_settings": {
"VCCLCompilerTool": {
'ExceptionHandling': 0, # /EHsc
'MultiProcessorCompilation': 'true',
'RuntimeTypeInfo': 'false',
'Optimization': 3, # full optimizations /O2 == /Og /Oi /Ot /Oy /Ob2 /GF /Gy
'StringPooling': 'true', # pool string literals
"AdditionalOptions": [
# C++ standard
"/std:c++<(STANDARD)",

# Optimizations
"/O2",
# "/Ob3", # aggressive inline
"/GL", # whole Program Optimization # /LTCG is implied with /GL.
"/DNDEBUG" # turn off asserts
],
}
} # MSVC
}, # Release
}, # configurations
} # target-defaults
}
11 changes: 7 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
"test:node": "mocha test/js/*.js",
"test:browser": "SUPERSTRING_USE_BROWSER_VERSION=1 mocha test/js/*.js",
"test": "npm run test:node && npm run test:browser",
"benchmark": "node benchmark/marker-index.benchmark.js",
"benchmark": "node benchmark/text-buffer.benchmark.js && node benchmark/marker-index.benchmark.js && node benchmark/large-text-buffer.benchmark.js",
"prepublishOnly": "git submodule update --init --recursive && npm run build:browser",
"standard": "standard --recursive src test"
"standard": "standard --recursive src test",
"format": "clang-format -i src/core/*.cc src/core/*.h src/bindings/*.cc src/bindings/*.h src/bindings/em/*.cc src/bindings/em/*.h",
"tidy": "clang-tidy src/core/*.cc src/core/*.h src/bindings/*.cc src/bindings/*.h src/bindings/em/*.cc src/bindings/em/*.h",
"tidy:fix": "npm run tidy -- --fix --fix-errors"
},
"repository": {
"type": "git",
Expand All @@ -35,11 +38,11 @@
},
"devDependencies": {
"chai": "^2.0.0",
"download": "^8.0.0",
"mocha": "^2.3.4",
"random-seed": "^0.2.0",
"standard": "^4.5.4",
"temp": "^0.8.3",
"unzip": "^0.1.11"
"temp": "^0.8.3"
},
"standard": {
"global": [
Expand Down
2 changes: 1 addition & 1 deletion src/bindings/bindings.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#include "marker-index-wrapper.h"
#include "nan.h"
#include <nan.h>
#include "patch-wrapper.h"
#include "range-wrapper.h"
#include "point-wrapper.h"
Expand Down
3 changes: 2 additions & 1 deletion src/bindings/em/auto-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
#include <emscripten/val.h>
#include "flat_set.h"
#include "marker-index.h"
#include "optional.h"
#include <optional>
using std::optional;
#include "point.h"
#include "text.h"

Expand Down
5 changes: 3 additions & 2 deletions src/bindings/marker-index-wrapper.cc
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#include "marker-index-wrapper.h"
#include <unordered_map>
#include "marker-index.h"
#include "nan.h"
#include <nan.h>
#include "noop.h"
#include "optional.h"
#include <optional>
using std::optional;
#include "point-wrapper.h"
#include "range.h"

Expand Down
7 changes: 4 additions & 3 deletions src/bindings/marker-index-wrapper.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "nan.h"
#include <nan.h>
#include "marker-index.h"
#include "optional.h"
#include <optional>
using std::optional;
#include "range.h"

class MarkerIndexWrapper : public Nan::ObjectWrap {
Expand Down Expand Up @@ -36,6 +37,6 @@ class MarkerIndexWrapper : public Nan::ObjectWrap {
static void find_ending_at(const Nan::FunctionCallbackInfo<v8::Value> &info);
static void find_boundaries_after(const Nan::FunctionCallbackInfo<v8::Value> &info);
static void dump(const Nan::FunctionCallbackInfo<v8::Value> &info);
MarkerIndexWrapper(unsigned seed);
MarkerIndex marker_index;
explicit MarkerIndexWrapper(unsigned seed);
};
2 changes: 1 addition & 1 deletion src/bindings/noop.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#pragma once

#include "nan.h"
#include <nan.h>

static void noop(const Nan::FunctionCallbackInfo<v8::Value>&) {}
5 changes: 3 additions & 2 deletions src/bindings/number-conversion.h
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#ifndef SUPERSTRING_NUMBER_CONVERSION_H
#define SUPERSTRING_NUMBER_CONVERSION_H

#include "nan.h"
#include "optional.h"
#include <nan.h>
#include <optional>
using std::optional;

namespace number_conversion {
template<typename T>
Expand Down
Loading