Skip to content

Commit 2fba112

Browse files
DeMoorJasperDeMoorJasper
authored
Merge pull request #44 from parcel-bundler/perf
Improve performance of addIndexedMappings
2 parents c97df2d + 2b5d361 commit 2fba112

14 files changed

+193
-88
lines changed

‎.vscode/settings.json

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"files.associations": {
3+
"__node_handle": "cpp",
4+
"__split_buffer": "cpp",
5+
"array": "cpp",
6+
"deque": "cpp",
7+
"iterator": "cpp",
8+
"queue": "cpp",
9+
"set": "cpp",
10+
"string": "cpp",
11+
"string_view": "cpp",
12+
"unordered_map": "cpp",
13+
"vector": "cpp",
14+
"__locale": "cpp",
15+
"__string": "cpp"
16+
}
17+
}

‎package.json

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@parcel/source-map",
3-
"version": "2.0.0-alpha.4.18",
3+
"version": "2.0.0-alpha.4.19",
44
"main": "./dist/node.js",
55
"types": "index.d.ts",
66
"browser": "./dist/wasm-browser.js",
@@ -23,7 +23,8 @@
2323
"lint": "prettier --write bench/run.js src/*.js",
2424
"typecheck": "flow",
2525
"format": "prettier --write \"./**/*.{js,md,mdx}\"",
26-
"compile-schema": "./flatc --cpp -o ./src ./src/sourcemap-schema.fbs"
26+
"compile-schema": "./flatc --cpp -o ./src ./src/sourcemap-schema.fbs",
27+
"clean": "rm -rf dist build"
2728
},
2829
"husky": {
2930
"hooks": {

‎src/MappingContainer.cpp

+5-11
Original file line numberDiff line numberDiff line change
@@ -487,19 +487,13 @@ void MappingContainer::addBufferMappings(const void *buf, int lineOffset, int co
487487
}
488488

489489
void MappingContainer::addIndexedMapping(int generatedLine, int generatedColumn, int originalLine, int originalColumn,
490-
std::string source, std::string name) {
490+
int sourceIndex, int nameIndex) {
491491
Position generatedPosition = Position{generatedLine, generatedColumn};
492492
Position originalPosition = Position{originalLine, originalColumn};
493-
int sourceIndex = -1;
494-
int nameIndex = -1;
495-
if (originalPosition.line > -1) {
496-
if (!source.empty()) {
497-
sourceIndex = addSource(source);
498-
}
499-
500-
if (!name.empty()) {
501-
nameIndex = addName(name);
502-
}
493+
494+
if (originalPosition.line < 0) {
495+
sourceIndex = -1;
496+
nameIndex = -1;
503497
}
504498

505499
addMapping(generatedPosition, originalPosition, sourceIndex, nameIndex);

‎src/MappingContainer.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class MappingContainer {
6464

6565
void offsetColumns(int line, int column, int columnOffset);
6666

67-
void addIndexedMapping(int generatedLine, int generatedColumn, int originalLine, int originalColumn, std::string source, std::string name);
67+
void addIndexedMapping(int generatedLine, int generatedColumn, int originalLine, int originalColumn, int sourceIndex, int nameIndex);
6868

6969
private:
7070
// Processed mappings, for all kinds of modifying within the sourcemap

‎src/SourceMap.js

+50-19
Original file line numberDiff line numberDiff line change
@@ -81,23 +81,57 @@ export default class SourceMap {
8181
* @param columnOffset an offset that gets added to the sourceColumn index of each mapping
8282
*/
8383
addIndexedMapping(mapping: IndexedMapping<string>, lineOffset?: number = 0, columnOffset?: number = 0): void {
84-
let hasValidOriginal =
85-
mapping.original &&
86-
typeof mapping.original.line === 'number' &&
87-
!isNaN(mapping.original.line) &&
88-
typeof mapping.original.column === 'number' &&
89-
!isNaN(mapping.original.column);
90-
91-
this.sourceMapInstance.addIndexedMapping(
92-
mapping.generated.line + lineOffset - 1,
93-
mapping.generated.column + columnOffset,
84+
// Not sure if it'll be worth it to add this back to C++, wrapping it in an array probably doesn't do that much harm in JS?
85+
// Also we barely use this function anyway...
86+
this.addIndexedMappings([mapping], lineOffset, columnOffset);
87+
}
88+
89+
_indexedMappingsToInt32Array(
90+
mappings: Array<IndexedMapping<string>>,
91+
lineOffset?: number = 0,
92+
columnOffset?: number = 0
93+
) {
94+
// Encode all mappings into a single typed array and make one call
95+
// to C++ instead of one for each mapping to improve performance.
96+
let mappingBuffer = new Int32Array(mappings.length * 6);
97+
let sources: Map<string, number> = new Map();
98+
let names: Map<string, number> = new Map();
99+
let i = 0;
100+
for (let mapping of mappings) {
101+
let hasValidOriginal =
102+
mapping.original &&
103+
typeof mapping.original.line === 'number' &&
104+
!isNaN(mapping.original.line) &&
105+
typeof mapping.original.column === 'number' &&
106+
!isNaN(mapping.original.column);
107+
108+
mappingBuffer[i++] = mapping.generated.line + lineOffset - 1;
109+
mappingBuffer[i++] = mapping.generated.column + columnOffset;
94110
// $FlowFixMe
95-
hasValidOriginal ? mapping.original.line - 1 : -1,
111+
mappingBuffer[i++] = hasValidOriginal ? mapping.original.line - 1 : -1;
96112
// $FlowFixMe
97-
hasValidOriginal ? mapping.original.column : -1,
98-
mapping.source ? relatifyPath(mapping.source, this.projectRoot) : '',
99-
mapping.name || ''
100-
);
113+
mappingBuffer[i++] = hasValidOriginal ? mapping.original.column : -1;
114+
115+
let sourceIndex = mapping.source ? sources.get(mapping.source) : -1;
116+
if (sourceIndex == null) {
117+
// $FlowFixMe
118+
sourceIndex = this.addSource(mapping.source);
119+
// $FlowFixMe
120+
sources.set(mapping.source, sourceIndex);
121+
}
122+
mappingBuffer[i++] = sourceIndex;
123+
124+
let nameIndex = mapping.name ? names.get(mapping.name) : -1;
125+
if (nameIndex == null) {
126+
// $FlowFixMe
127+
nameIndex = this.addName(mapping.name);
128+
// $FlowFixMe
129+
names.set(mapping.name, nameIndex);
130+
}
131+
mappingBuffer[i++] = nameIndex;
132+
}
133+
134+
return mappingBuffer;
101135
}
102136

103137
/**
@@ -116,10 +150,7 @@ export default class SourceMap {
116150
lineOffset?: number = 0,
117151
columnOffset?: number = 0
118152
): SourceMap {
119-
for (let mapping of mappings) {
120-
this.addIndexedMapping(mapping, lineOffset, columnOffset);
121-
}
122-
return this;
153+
throw new Error('Should be implemented by child class');
123154
}
124155

125156
/**

‎src/napi/SourceMap.cpp

+22-12
Original file line numberDiff line numberDiff line change
@@ -224,24 +224,34 @@ Napi::Value SourceMapBinding::getMap(const Napi::CallbackInfo &info) {
224224
return obj;
225225
}
226226

227-
// addIndexedMapping(generatedLine, generatedColumn, originalLine, originalColumn, source, name)
228-
void SourceMapBinding::addIndexedMapping(const Napi::CallbackInfo &info) {
227+
void SourceMapBinding::addIndexedMappings(const Napi::CallbackInfo &info) {
229228
Napi::Env env = info.Env();
230229
Napi::HandleScope scope(env);
231230

232-
if (info.Length() < 6) {
233-
Napi::TypeError::New(env, "Expected 6 parameters").ThrowAsJavaScriptException();
231+
if (info.Length() < 1) {
232+
Napi::TypeError::New(env, "Expected 1 Parameter").ThrowAsJavaScriptException();
234233
return;
235234
}
236235

237-
int generatedLine = info[0].As<Napi::Number>().Int32Value();
238-
int generatedColumn = info[1].As<Napi::Number>().Int32Value();
239-
int originalLine = info[2].As<Napi::Number>().Int32Value();
240-
int originalColumn = info[3].As<Napi::Number>().Int32Value();
241-
std::string source = info[4].As<Napi::String>().Utf8Value();
242-
std::string name = info[5].As<Napi::String>().Utf8Value();
236+
if (!info[0].IsTypedArray()) {
237+
Napi::TypeError::New(env, "Expected a typed array for the first parameter").ThrowAsJavaScriptException();
238+
return;
239+
}
243240

244-
_mapping_container.addIndexedMapping(generatedLine, generatedColumn, originalLine, originalColumn, source, name);
241+
auto buffer = info[0].As<Napi::TypedArrayOf<int32_t>>();
242+
unsigned int length = buffer.ElementLength();
243+
for (unsigned int i = 0; i < length; i += 6) {
244+
unsigned int mappingIndex = i / 6;
245+
246+
_mapping_container.addIndexedMapping(
247+
buffer[i],
248+
buffer[i + 1],
249+
buffer[i + 2],
250+
buffer[i + 3],
251+
buffer[i + 4],
252+
buffer[i + 5]
253+
);
254+
}
245255
}
246256

247257
Napi::Value SourceMapBinding::getSourceIndex(const Napi::CallbackInfo &info) {
@@ -455,7 +465,7 @@ Napi::Object SourceMapBinding::Init(Napi::Env env, Napi::Object exports) {
455465
InstanceMethod("stringify", &SourceMapBinding::stringify),
456466
InstanceMethod("toBuffer", &SourceMapBinding::toBuffer),
457467
InstanceMethod("getMap", &SourceMapBinding::getMap),
458-
InstanceMethod("addIndexedMapping", &SourceMapBinding::addIndexedMapping),
468+
InstanceMethod("addIndexedMappings", &SourceMapBinding::addIndexedMappings),
459469
InstanceMethod("addName", &SourceMapBinding::addName),
460470
InstanceMethod("addSource", &SourceMapBinding::addSource),
461471
InstanceMethod("setSourceContent", &SourceMapBinding::setSourceContent),

‎src/napi/SourceMap.h

-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ class SourceMapBinding : public Napi::ObjectWrap<SourceMapBinding> {
1515

1616
void addBufferMappings(const Napi::CallbackInfo &info);
1717

18-
void addIndexedMapping(const Napi::CallbackInfo &info);
19-
2018
void addIndexedMappings(const Napi::CallbackInfo &info);
2119

2220
void extends(const Napi::CallbackInfo &info);

‎src/node.js

+10
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ export default class NodeSourceMap extends SourceMap {
3030
return this;
3131
}
3232

33+
addIndexedMappings(
34+
mappings: Array<IndexedMapping<string>>,
35+
lineOffset?: number = 0,
36+
columnOffset?: number = 0
37+
): SourceMap {
38+
let mappingBuffer = this._indexedMappingsToInt32Array(mappings, lineOffset, columnOffset);
39+
this.sourceMapInstance.addIndexedMappings(mappingBuffer);
40+
return this;
41+
}
42+
3343
toBuffer(): Buffer {
3444
return this.sourceMapInstance.toBuffer();
3545
}

‎src/utils.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ export function isAbsolute(filepath: string): boolean {
1717
}
1818

1919
export function normalizePath(filepath: string): string {
20-
return filepath.replace(ABSOLUTE_PATH_REGEX, '/').replace(PATH_SEPARATOR_REGEX, '/');
20+
if (process.platform === 'win32') {
21+
return filepath.replace(ABSOLUTE_PATH_REGEX, '/').replace(PATH_SEPARATOR_REGEX, '/');
22+
}
23+
24+
return filepath;
2125
}
2226

2327
export function relatifyPath(filepath: string, rootDir: string): string {

‎src/wasm.js

+14-2
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ function patchMapping(mapping: any): any {
2828
return mapping;
2929
}
3030

31-
function arrayToEmbind(Type, from): any {
32-
let arr = new Module.VectorString();
31+
function arrayToEmbind(VectorType, from): any {
32+
let arr = new VectorType();
3333
for (let v of from) {
3434
arr.push_back(v);
3535
}
@@ -82,6 +82,18 @@ export default class WasmSourceMap extends SourceMap {
8282
return this;
8383
}
8484

85+
addIndexedMappings(
86+
mappings: Array<IndexedMapping<string>>,
87+
lineOffset?: number = 0,
88+
columnOffset?: number = 0
89+
): SourceMap {
90+
let mappingBuffer = this._indexedMappingsToInt32Array(mappings, lineOffset, columnOffset);
91+
let mappingBufferArray = arrayToEmbind(Module.VectorInt, mappingBuffer);
92+
this.sourceMapInstance.addIndexedMappings(mappingBufferArray);
93+
mappingBufferArray.delete();
94+
return this;
95+
}
96+
8597
findClosestMapping(line: number, column: number): ?IndexedMapping<string> {
8698
let mapping = this.sourceMapInstance.findClosestMapping(line, column);
8799
if (mapping.generated.line === -1) {

‎src/wasm/SourceMap.cpp

+16-4
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,21 @@ std::vector<Mapping> SourceMap::getMappings(){
6262
return mappings;
6363
}
6464

65-
// addIndexedMapping(generatedLine, generatedColumn, originalLine, originalColumn, source, name)
66-
void SourceMap::addIndexedMapping(int generatedLine, int generatedColumn, int originalLine, int originalColumn, std::string source, std::string name) {
67-
_mapping_container.addIndexedMapping(generatedLine, generatedColumn, originalLine, originalColumn, source, name);
65+
// addIndexedMappings(mappings: Vector<Int32>)
66+
void SourceMap::addIndexedMappings(std::vector<int> buffer) {
67+
unsigned int length = buffer.size();
68+
for (unsigned int i = 0; i < length; i += 6) {
69+
unsigned int mappingIndex = i / 6;
70+
71+
_mapping_container.addIndexedMapping(
72+
buffer[i],
73+
buffer[i + 1],
74+
buffer[i + 2],
75+
buffer[i + 3],
76+
buffer[i + 4],
77+
buffer[i + 5]
78+
);
79+
}
6880
}
6981

7082
int SourceMap::getSourceIndex(std::string source) {
@@ -147,7 +159,7 @@ EMSCRIPTEN_BINDINGS(my_class_example) {
147159
.constructor<>()
148160
.function("addRawMappings", &SourceMap::addRawMappings)
149161
.function("addBufferMappings", &SourceMap::addBufferMappings)
150-
.function("addIndexedMapping", &SourceMap::addIndexedMapping)
162+
.function("addIndexedMappings", &SourceMap::addIndexedMappings)
151163
.function("getVLQMappings", &SourceMap::getVLQMappings)
152164
.function("getMappings", &SourceMap::getMappings)
153165
.function("getSources", &SourceMap::getSources)

‎src/wasm/SourceMap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class SourceMap {
99

1010
void addRawMappings(std::string rawMappings, std::vector<std::string> sources, std::vector<std::string> sourcesContent, std::vector<std::string> names, int lineOffset, int columnOffset);
1111
void addBufferMappings(std::string mapbuffer, int lineOffset, int columnOffset);
12-
void addIndexedMapping(int generatedLine, int generatedColumn, int originalLine, int originalColumn, std::string source, std::string name);
12+
void addIndexedMappings(std::vector<int> buffer);
1313

1414
void extends(std::string mapBuffer);
1515

‎test/formats.test.js

+23-11
Original file line numberDiff line numberDiff line change
@@ -78,20 +78,32 @@ describe('SourceMap - Formats', () => {
7878
assert.deepEqual(stringifiedMap.sources, ['./helloworld.coffee']);
7979
});
8080

81-
it('Should make all sourcePaths web friendly aka no windows backslashes', async () => {
82-
let map = new SourceMap('C:\\Users\\test\\');
83-
map.addRawMappings({
84-
mappings: SIMPLE_SOURCE_MAP.mappings,
85-
sources: ['C:\\Users\\test\\helloworld.coffee'],
86-
names: SIMPLE_SOURCE_MAP.names,
81+
describe('win32', function () {
82+
let platform = process.platform;
83+
84+
before(() => {
85+
Object.defineProperty(process, 'platform', { value: 'win32' });
8786
});
8887

89-
let stringifiedMap = await map.stringify({
90-
file: 'index.js.map',
91-
sourceRoot: '/',
92-
format: 'object',
88+
after(() => {
89+
Object.defineProperty(process, 'platform', { value: platform });
9390
});
9491

95-
assert.deepEqual(stringifiedMap.sources, ['./helloworld.coffee']);
92+
it('Should make all sourcePaths web friendly aka no windows backslashes', async () => {
93+
let map = new SourceMap('C:\\Users\\test\\');
94+
map.addRawMappings({
95+
mappings: SIMPLE_SOURCE_MAP.mappings,
96+
sources: ['C:\\Users\\test\\helloworld.coffee'],
97+
names: SIMPLE_SOURCE_MAP.names,
98+
});
99+
100+
let stringifiedMap = await map.stringify({
101+
file: 'index.js.map',
102+
sourceRoot: '/',
103+
format: 'object',
104+
});
105+
106+
assert.deepEqual(stringifiedMap.sources, ['./helloworld.coffee']);
107+
});
96108
});
97109
});

0 commit comments

Comments
 (0)