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.

Sign up for GitHub

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

add option decodeMappings to remapping #88

Closed
wants to merge 3 commits into from
Closed

add option decodeMappings to remapping #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2819f98
add option decodeMappings to remapping
milahu Sep 28, 2020
4c25d9c
add option segmentsAreSorted
milahu Sep 28, 2020
fbc7722
rename decodeMappings to skipEncodeMappings, refactor
milahu Oct 5, 2020
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
14 changes: 10 additions & 4 deletions src/build-source-map-tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import OriginalSource from './original-source';
import resolve from './resolve';
import SourceMapTree from './source-map-tree';
import stripFilename from './strip-filename';
import { SourceMapInput, SourceMapLoader } from './types';
import { DecodedSourceMap, SourceMapInput, SourceMapLoader } from './types';

function asArray<T>(value: T | T[]): T[] {
if (Array.isArray(value)) return value;
Expand All @@ -40,9 +40,15 @@ function asArray<T>(value: T | T[]): T[] {
export default function buildSourceMapTree(
input: SourceMapInput | SourceMapInput[],
loader: SourceMapLoader,
relativeRoot?: string
relativeRoot?: string,
segmentsAreSorted?: boolean
): SourceMapTree {
const maps = asArray(input).map(decodeSourceMap);
const maps = asArray(input).map(
(map: SourceMapInput): DecodedSourceMap => {
return decodeSourceMap(map, segmentsAreSorted);
}
);
// as DecodedSourceMap[];
const map = maps.pop()!;

for (let i = 0; i < maps.length; i++) {
Expand Down Expand Up @@ -78,7 +84,7 @@ export default function buildSourceMapTree(

// Else, it's a real sourcemap, and we need to recurse into it to load its
// source files.
return buildSourceMapTree(decodeSourceMap(sourceMap), loader, uri);
return buildSourceMapTree(decodeSourceMap(sourceMap, segmentsAreSorted), loader, uri);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that we don't forward segmentsAreSorted to the recursive buildSourceMapTree (4th param). Which makes me a bit hesitant. I'm not sure we should allow an options that gives a broken result if misused.

What if instead we just intelligently detect if the segmentLine is sorted? It should allow us to be pretty fast overall (certainly faster than cloning and sorting always), and we'd be guaranteed to get correct results even if a sourcemap is improperly sorted.

// src/decode-source-map.ts
/**
 * Decodes an input sourcemap into a `DecodedSourceMap` sourcemap object.
 *
 * Valid input maps include a `DecodedSourceMap`, a `RawSourceMap`, or JSON
 * representations of either type.
 */
export default function decodeSourceMap(map: SourceMapInput): DecodedSourceMap {
  let needsClone = true;
  let sorted = true;

  if (typeof map === 'string') {
    needsClone = false;
    map = JSON.parse(map) as DecodedSourceMap | RawSourceMap;
  }

  let { mappings } = map;
  if (typeof mappings === 'string') {
    needsClone = false;
    mappings = decode(mappings);
  }

  for (let i = 0; i < mappings.length; i++) {
    const line = mappings[i];
    for (let j = 0; j < line.length - 1; j++) {
      if (line[j] > line[j + 1]) {
        sorted = false;
        break;
      }
    }
  }

  // Sort each Line's segments if needed. There's no guarantee that segments are
  // sorted for us, and even Chrome's implementation sorts:
  // https://cs.chromium.org/chromium/src/third_party/devtools-frontend/src/front_end/sdk/SourceMap.js?l=507-508&rcl=109232bcf479c8f4ef8ead3cf56c49eb25f8c2f0
  if (!sorted) {
    if (needsClone) {
      // Clone the Line so that we can sort it. We don't want to mutate an array
      // that we don't own directly.
      mappings = mappings.map(cloneAndSortSegmentLine);
    } else {
      mappings.forEach(sortSegments);
    }
  }

  return defaults({ mappings }, map);
}

function cloneAndSortSegmentLine(segments: SourceMapSegment[]): SourceMapSegment[] {
  return sortSegments(segments.slice());
}

function sortSegments(segments: SourceMapSegment[]): SourceMapSegment[] {
  return segments.sort(segmentComparator);
}

function segmentComparator(a: SourceMapSegment, b: SourceMapSegment): number {
  return a[0] - b[0];
}

Copy link
Contributor Author

@milahu milahu Oct 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if instead we just intelligently detect if the segmentLine is sorted?

looping all lines and all segments sounds like expensive overhead, if everything is sorted (you also need to break the outer for loop)

more something like .... 'check if the first 100 segments are sorted, if yes, assume the rest is sorted too'

one problem with that approach is:
sourcemaps can be concatted, so the result can be a mix of sorted and unsorted, which would give a false positive (segments are sorted) and a broken result

we could take random samples - random inputs require random strategies - so we can at least minimize the risk of false positives

still, under very very bad conditions, there will always be false positives and broken results, so i would keep the segmentsAreSorted option - only the user knows his data, and his sourcemap generators. if he wants to use this optimization, he must make sure the 'segments are sorted'

similar problem: detect low-resolution sourcemaps - this is simply not possible in a cheap way, and we would have to tokenize the source string, and check how many tokens are mapped

see unix philosophy - focus on one task

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looping all lines and all segments sounds like expensive overhead, if everything is sorted (you also need to break the outer for loop)

Compared to doing nothing, it is expensive. But compared to the rest of the work we're doing (tracing the segment through multiple sourcemaps), it's still very cheap to do. https://jsbench.github.io/#b5130ae558bbca6b002025aca004201e has checking the full sourcemap's sort at 14x faster than doing the binarySearch tracing for just 1/8th of a map. So 112x faster than the rest of the work.

And sorting an already sorted mapping is itself faster than the tracing. I think we this is still a huge speed improvement over the current sorting code, and the correctness outweighs the cost we'd pay.

Copy link
Contributor Author

@milahu milahu Oct 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the benchmark

Compared to doing nothing, it is expensive.

still, that is the special case i want to optimize with segmentsAreSorted. lets have both, no? even if 'check if sorted + sort' is slower than 'sort'

why is tracing not done like this? this is 10 times faster than binary search
edit: the input data was too simple (see my next comment)

caching a space-time tradeoff but that should be fine, since sourcemaps are usually small (max a few megabytes)

// tracing 1/8th of the map -- via cache array

const cache_array = [];
for (let L = 0; L < mappings.length; L++) {
    const line = mappings[L];
    cache_array.push([]);
    const cache_line = cache_array[cache_array.length - 1];
    for (let S = 0; S < line.length; S++) {
        const seg = line[S];
        cache_line[seg[0]] = seg; // pass array by reference
    }
}

for (let i = 0; i < 25; i++) {
    const line = cache_array[i];
    for (let j = 0; j < 250; j++) {
        line[j];
    }
}

this works, cos javascript allows empty items in arrays, so we can do 'random access write'

Copy link
Contributor Author

@milahu milahu Oct 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is tracing not done like this?

found the answer:
the input data was too simple: dense columns, only one value

with sparse columns and more values, binarySearch is fastest

 const mappings = [];
 for (let i = 0; i < 100; i++) {
     const line = [];
     for (let j = 0; j < 1000; j++) {
-        line.push([i]);
+        line.push([j*10, 0, i+10, j]); // output_column, source, line, column
     }
     mappings.push(line);
 }

but still, binarySearch can be optimized for sequential search, see this benchmark

});

let tree = new SourceMapTree(map, children);
Expand Down
17 changes: 11 additions & 6 deletions src/decode-source-map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,28 @@ import { DecodedSourceMap, RawSourceMap, SourceMapInput, SourceMapSegment } from
* Valid input maps include a `DecodedSourceMap`, a `RawSourceMap`, or JSON
* representations of either type.
*/
export default function decodeSourceMap(map: SourceMapInput): DecodedSourceMap {
export default function decodeSourceMap(
map: SourceMapInput,
segmentsAreSorted?: boolean
): DecodedSourceMap {
if (typeof map === 'string') {
map = JSON.parse(map) as DecodedSourceMap | RawSourceMap;
}

let { mappings } = map;
if (typeof mappings === 'string') {
mappings = decode(mappings);
} else {
} else if (!segmentsAreSorted) {
// Clone the Line so that we can sort it. We don't want to mutate an array
// that we don't own directly.
mappings = mappings.map(cloneSegmentLine);
}
// Sort each Line's segments. There's no guarantee that segments are sorted for us,
// and even Chrome's implementation sorts:
// https://cs.chromium.org/chromium/src/third_party/devtools-frontend/src/front_end/sdk/SourceMap.js?l=507-508&rcl=109232bcf479c8f4ef8ead3cf56c49eb25f8c2f0
mappings.forEach(sortSegments);
if (!segmentsAreSorted) {
// Sort each Line's segments. There's no guarantee that segments are sorted for us,
// and even Chrome's implementation sorts:
// https://cs.chromium.org/chromium/src/third_party/devtools-frontend/src/front_end/sdk/SourceMap.js?l=507-508&rcl=109232bcf479c8f4ef8ead3cf56c49eb25f8c2f0
mappings.forEach(sortSegments);
}

return defaults({ mappings }, map);
}
Expand Down
37 changes: 30 additions & 7 deletions src/remapping.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@

import buildSourceMapTree from './build-source-map-tree';
import SourceMap from './source-map';
import { SourceMapInput, SourceMapLoader } from './types';
import {
DecodedSourceMap,
RawOrDecodedSourceMap,
RawSourceMap,
SourceMapInput,
SourceMapLoader,
} from './types';

/**
* Traces through all the mappings in the root sourcemap, through the sources
Expand All @@ -27,14 +33,31 @@ import { SourceMapInput, SourceMapLoader } from './types';
* it returns a falsey value, that source file is treated as an original,
* unmodified source file.
*
* Pass `excludeContent` content to exclude any self-containing source file
* content from the output sourcemap.
* Pass `excludeContent` to exclude any self-containing source file content
* from the output sourcemap.
*
* Pass `skipEncodeMappings` to get a sourcemap with decoded mappings.
*
* Pass `segmentsAreSorted` to skip sorting of segments. Only use when segments
* are guaranteed to be sorted, otherwise mappings will be wrong.
*/
export default function remapping(
input: SourceMapInput | SourceMapInput[],
loader: SourceMapLoader,
excludeContent?: boolean
): SourceMap {
const graph = buildSourceMapTree(input, loader);
return new SourceMap(graph.traceMappings(), !!excludeContent);
options?: {
excludeContent?: boolean;
skipEncodeMappings?: boolean;
segmentsAreSorted?: boolean;
}
): RawOrDecodedSourceMap {
// TODO remove in future
// tslint:disable-next-line:strict-type-predicates
if (typeof options === 'boolean') throw new Error('Please use the new API');

const graph = buildSourceMapTree(input, loader, '', !!(options && options.segmentsAreSorted));
return new SourceMap(
graph.traceMappings(),
!!(options && options.excludeContent),
!!(options && options.skipEncodeMappings)
);
}
10 changes: 5 additions & 5 deletions src/source-map.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,25 @@
*/

import { encode } from 'sourcemap-codec';
import { DecodedSourceMap, RawSourceMap } from './types';
import { DecodedSourceMap, RawOrDecodedSourceMap, SourceMapSegment } from './types';

/**
* A SourceMap v3 compatible sourcemap, which only includes fields that were
* provided to it.
*/
export default class SourceMap implements RawSourceMap {
export default class SourceMap implements RawOrDecodedSourceMap {
file?: string | null;
mappings: string;
mappings: string | SourceMapSegment[][];
sourceRoot?: string;
names: string[];
sources: (string | null)[];
sourcesContent?: (string | null)[];
version: 3;

constructor(map: DecodedSourceMap, excludeContent: boolean) {
constructor(map: DecodedSourceMap, excludeContent: boolean, skipEncodeMappings: boolean) {
this.version = 3; // SourceMap spec says this should be first.
if ('file' in map) this.file = map.file;
this.mappings = encode(map.mappings);
this.mappings = skipEncodeMappings ? map.mappings : encode(map.mappings);
this.names = map.names;

// TODO: We first need to make all source URIs relative to the sourceRoot
Expand Down
4 changes: 4 additions & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ export interface DecodedSourceMap extends SourceMapV3 {
mappings: SourceMapSegment[][];
}

export interface RawOrDecodedSourceMap extends SourceMapV3 {
mappings: string | SourceMapSegment[][];
}

export interface SourceMapSegmentObject {
column: number;
line: number;
Expand Down
151 changes: 139 additions & 12 deletions test/unit/remapping.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,42 @@
*/

import remapping from '../../src/remapping';
import { RawSourceMap } from '../../src/types';
import { DecodedSourceMap, RawSourceMap } from '../../src/types';

describe('remapping', () => {
// transform chain:
// 1+1 \n 1+1 \n\n 1 + 1;
// line 0 column 0 < line 1 column 1 < line 2 column 2
// transpiled.min.js < transpiled.js < helloworld.js
// v v
// rawMap transpiledMap
// v
// translatedMap

// segment = output_column [, source, line, column [, name]]
// all decoded numbers are zero-based

const rawMap: RawSourceMap = {
file: 'transpiled.min.js',
// 0th column of 1st line of output file translates into the 1st source
// file, line 2, column 1, using 1st name.
mappings: 'AACCA',
// line 0, column 0 <- source 0, line 1, column 1, name 0
mappings: 'AACCA', // [[[ 0, 0, 1, 1, 0 ]]]
names: ['add'],
sources: ['transpiled.js'],
sourcesContent: ['1+1'],
sourcesContent: ['\n 1+1'],
version: 3,
};
const transpiledMap: RawSourceMap = {
// 1st column of 2nd line of output file translates into the 1st source
// file, line 3, column 2
mappings: ';CAEE',
// line 1, column 1 <- source 0, line 2, column 2
mappings: ';CAEE', // [ [], [[ 1, 0, 2, 2 ]] ]
names: [],
sources: ['helloworld.js'],
sourcesContent: ['\n\n 1 + 1;'],
version: 3,
};
const translatedMap: RawSourceMap = {
file: 'transpiled.min.js',
// 0th column of 1st line of output file translates into the 1st source
// file, line 3, column 2, using first name
mappings: 'AAEEA',
// line 0, column 0 <- source 0, line 2, column 2, name 0
mappings: 'AAEEA', // [[[ 0, 0, 2, 2, 0 ]]]
names: ['add'],
// TODO: support sourceRoot
// sourceRoot: '',
Expand All @@ -50,6 +59,62 @@ describe('remapping', () => {
version: 3,
};

const rawMapDecoded: DecodedSourceMap = {
...rawMap,
mappings: [[[0, 0, 1, 1, 0]]],
};
const transpiledMapDecoded: DecodedSourceMap = {
...transpiledMap,
mappings: [[], [[1, 0, 2, 2]]],
};
const translatedMapDecoded: DecodedSourceMap = {
...translatedMap,
mappings: [[[0, 0, 2, 2, 0]]],
};

// segments in reverse order to test `segmentsAreSorted` option
// sort order is preserved in result
// transform chain:
// line 0 column 0 < line 1 column 1 < line 2 column 2
// line 0 column 1 < line 1 column 2 < line 2 column 1
// transpiled.min.js < transpiled.js < helloworld.js
// v v
// rawMap transpiledMap
const rawMapDecodedReversed: DecodedSourceMap = {
...rawMap,
// line 0, column 1 <- source 0, line 1, column 2, name 0
// line 0, column 0 <- source 0, line 1, column 1
mappings: [
[
[1, 0, 1, 2, 0],
[0, 0, 1, 1],
],
],
};
const transpiledMapDecodedReversed: DecodedSourceMap = {
...transpiledMap,
// line 1, column 2 <- source 0, line 2, column 1
// line 1, column 1 <- source 0, line 2, column 2
mappings: [
[],
[
[2, 0, 2, 1],
[1, 0, 2, 2],
],
],
};
const translatedMapDecodedReversed: DecodedSourceMap = {
...translatedMap,
// line 0, column 1 <- source 0, line 2, column 1, name 0
// line 0, column 0 <- source 0, line 2, column 2
mappings: [
[
[1, 0, 2, 1, 0],
[0, 0, 2, 2],
],
],
};

test('does not alter a lone sourcemap', () => {
const map = remapping(rawMap, () => null);
expect(map).toEqual(rawMap);
Expand Down Expand Up @@ -150,9 +215,71 @@ describe('remapping', () => {
return transpiledMap;
}
},
true
{
excludeContent: true,
}
);

expect(map).not.toHaveProperty('sourcesContent');
});

test('returns decoded mappings if `skipEncodeMappings` is set', () => {
const map = remapping(
rawMap,
(name: string) => {
if (name === 'transpiled.js') {
return transpiledMap;
}
},
{
skipEncodeMappings: true,
}
);

expect(map).toEqual(translatedMapDecoded);
});

test('accepts decoded mappings as input', () => {
const map = remapping(rawMapDecoded, (name: string) => {
if (name === 'transpiled.js') {
return transpiledMapDecoded;
}
});

expect(map).toEqual(translatedMap);
});

test('skips sorting of segments if `segmentsAreSorted` is set', () => {
const map = remapping(
rawMapDecodedReversed,
(name: string) => {
if (name === 'transpiled.js') {
return transpiledMapDecodedReversed;
}
},
{
skipEncodeMappings: true,
segmentsAreSorted: true,
}
);
expect(map).toEqual(translatedMapDecodedReversed);
});

test('throws error when old API is used', () => {
try {
remapping(
rawMapDecoded,
(name: string) => {
if (name === 'transpiled.js') {
return transpiledMapDecoded;
}
},
true as any // old API
);
// fail
expect(1).toEqual(0);
} catch (error) {
expect(error.message).toEqual('Please use the new API');
}
});
});
Loading