Skip to content

Commit

Permalink
Use yieldy version of sourceMapString in server
Browse files Browse the repository at this point in the history
Summary:
Changelog:
* **[Performance]:** Make server more responsive during source map serialization.

We have an existing (currently unused) non-blocking implementation of `sourceMapObject` that relies on explicitly chunking the source map encoding work. Here we reuse the same underlying machinery to make `sourceMapString` non-blocking, as a cheap way of reducing long main-thread pauses during `.map` requests.

NOTE: `.map` requests do have a significant I/O component today, which isn't really addressed here. The I/O is best eliminated by passing the `excludeSource=true` parameter, typically in combination with `sourcePaths=url-server` (see D56952063, D56952065).

Reviewed By: robhogan

Differential Revision: D56957754

fbshipit-source-id: b18fd1d801d4d4542e39425fddd8cdb5ed8d6ce1
  • Loading branch information
motiz88 authored and facebook-github-bot committed May 8, 2024
1 parent 788ff0d commit 34148e6
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 108 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import type {Module} from '../../types.flow';

import CountingSet from '../../../lib/CountingSet';

const sourceMapString = require('../sourceMapString');
const {
sourceMapString,
sourceMapStringNonBlocking,
} = require('../sourceMapString');

const polyfill: Module<> = {
path: '/root/pre.js',
Expand Down Expand Up @@ -77,102 +80,119 @@ const barModule: Module<> = {
],
};

it('should serialize a very simple bundle', () => {
expect(
JSON.parse(
sourceMapString([polyfill, fooModule, barModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => false,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/pre.js', '/root/foo.js', '/root/bar.js'],
sourcesContent: ['source pre', 'source foo', 'source bar'],
x_facebook_sources: [null, [{names: ['<global>'], mappings: 'AAA'}], null],
names: [],
mappings: '',
});
});
describe.each([sourceMapString, sourceMapStringNonBlocking])(
'%p',
sourceMapStringImpl => {
test('should serialize a very simple bundle', async () => {
expect(
JSON.parse(
await sourceMapStringImpl([polyfill, fooModule, barModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => false,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/pre.js', '/root/foo.js', '/root/bar.js'],
sourcesContent: ['source pre', 'source foo', 'source bar'],
x_facebook_sources: [
null,
[{names: ['<global>'], mappings: 'AAA'}],
null,
],
names: [],
mappings: '',
});
});

it('modules should appear in their original order', () => {
expect(
JSON.parse(
sourceMapString([polyfill, barModule, fooModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => false,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/pre.js', '/root/bar.js', '/root/foo.js'],
sourcesContent: ['source pre', 'source bar', 'source foo'],
x_facebook_sources: [null, null, [{names: ['<global>'], mappings: 'AAA'}]],
names: [],
mappings: '',
});
});
test('modules should appear in their original order', async () => {
expect(
JSON.parse(
await sourceMapStringImpl([polyfill, barModule, fooModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => false,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/pre.js', '/root/bar.js', '/root/foo.js'],
sourcesContent: ['source pre', 'source bar', 'source foo'],
x_facebook_sources: [
null,
null,
[{names: ['<global>'], mappings: 'AAA'}],
],
names: [],
mappings: '',
});
});

it('should not include the source of an asset', () => {
const assetModule: Module<> = {
path: '/root/asset.jpg',
dependencies: new Map(),
inverseDependencies: new CountingSet(),
getSource: () => {
throw new Error('should not read the source of an asset');
},
output: [
{
type: 'js/module/asset',
data: {
code: '__d(function() {/* code for bar */});',
lineCount: 1,
map: [],
test('should not include the source of an asset', async () => {
const assetModule: Module<> = {
path: '/root/asset.jpg',
dependencies: new Map(),
inverseDependencies: new CountingSet(),
getSource: () => {
throw new Error('should not read the source of an asset');
},
},
],
};
output: [
{
type: 'js/module/asset',
data: {
code: '__d(function() {/* code for bar */});',
lineCount: 1,
map: [],
},
},
],
};

expect(
JSON.parse(
sourceMapString([fooModule, assetModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => false,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/foo.js', '/root/asset.jpg'],
sourcesContent: ['source foo', ''],
x_facebook_sources: [[{names: ['<global>'], mappings: 'AAA'}], null],
names: [],
mappings: '',
});
});
expect(
JSON.parse(
await sourceMapStringImpl([fooModule, assetModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => false,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/foo.js', '/root/asset.jpg'],
sourcesContent: ['source foo', ''],
x_facebook_sources: [[{names: ['<global>'], mappings: 'AAA'}], null],
names: [],
mappings: '',
});
});

it('should emit x_google_ignoreList based on shouldAddToIgnoreList', () => {
expect(
JSON.parse(
sourceMapString([polyfill, fooModule, barModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => true,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/pre.js', '/root/foo.js', '/root/bar.js'],
sourcesContent: ['source pre', 'source foo', 'source bar'],
x_facebook_sources: [null, [{names: ['<global>'], mappings: 'AAA'}], null],
names: [],
mappings: '',
x_google_ignoreList: [0, 1, 2],
});
});
test('should emit x_google_ignoreList based on shouldAddToIgnoreList', async () => {
expect(
JSON.parse(
await sourceMapStringImpl([polyfill, fooModule, barModule], {
excludeSource: false,
processModuleFilter: module => true,
shouldAddToIgnoreList: module => true,
getSourceUrl: null,
}),
),
).toEqual({
version: 3,
sources: ['/root/pre.js', '/root/foo.js', '/root/bar.js'],
sourcesContent: ['source pre', 'source foo', 'source bar'],
x_facebook_sources: [
null,
[{names: ['<global>'], mappings: 'AAA'}],
null,
],
names: [],
mappings: '',
x_google_ignoreList: [0, 1, 2],
});
});
},
);
20 changes: 18 additions & 2 deletions packages/metro/src/DeltaBundler/Serializers/sourceMapString.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
import type {Module} from '../types.flow';
import type {SourceMapGeneratorOptions} from './sourceMapGenerator';

const {sourceMapGenerator} = require('./sourceMapGenerator');
const {
sourceMapGenerator,
sourceMapGeneratorNonBlocking,
} = require('./sourceMapGenerator');

function sourceMapString(
modules: $ReadOnlyArray<Module<>>,
Expand All @@ -25,4 +28,17 @@ function sourceMapString(
});
}

module.exports = sourceMapString;
async function sourceMapStringNonBlocking(
modules: $ReadOnlyArray<Module<>>,
options: SourceMapGeneratorOptions,
): Promise<string> {
const generator = await sourceMapGeneratorNonBlocking(modules, options);
return generator.toString(undefined, {
excludeSource: options.excludeSource,
});
}

module.exports = {
sourceMapString,
sourceMapStringNonBlocking,
};
25 changes: 15 additions & 10 deletions packages/metro/src/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ const {
getExplodedSourceMap,
} = require('./DeltaBundler/Serializers/getExplodedSourceMap');
const getRamBundleInfo = require('./DeltaBundler/Serializers/getRamBundleInfo');
const sourceMapString = require('./DeltaBundler/Serializers/sourceMapString');
const {
sourceMapStringNonBlocking,
} = require('./DeltaBundler/Serializers/sourceMapString');
const IncrementalBundler = require('./IncrementalBundler');
const ResourceNotFoundError = require('./IncrementalBundler/ResourceNotFoundError');
const bundleToString = require('./lib/bundleToString');
Expand Down Expand Up @@ -288,7 +290,7 @@ class Server {
).code;
}
if (!bundleMap) {
bundleMap = sourceMapString(
bundleMap = await sourceMapStringNonBlocking(
[...prepend, ...this._getSortedModules(graph)],
{
excludeSource: serializerOptions.excludeSource,
Expand Down Expand Up @@ -1151,14 +1153,17 @@ class Server {
prepend = [];
}

return sourceMapString([...prepend, ...this._getSortedModules(graph)], {
excludeSource: serializerOptions.excludeSource,
processModuleFilter: this._config.serializer.processModuleFilter,
shouldAddToIgnoreList: (module: Module<>) =>
this._shouldAddModuleToIgnoreList(module),
getSourceUrl: (module: Module<>) =>
this._getModuleSourceUrl(module, serializerOptions.sourcePaths),
});
return await sourceMapStringNonBlocking(
[...prepend, ...this._getSortedModules(graph)],
{
excludeSource: serializerOptions.excludeSource,
processModuleFilter: this._config.serializer.processModuleFilter,
shouldAddToIgnoreList: (module: Module<>) =>
this._shouldAddModuleToIgnoreList(module),
getSourceUrl: (module: Module<>) =>
this._getModuleSourceUrl(module, serializerOptions.sourcePaths),
},
);
},
finish({mres, result}) {
mres.setHeader('Content-Type', 'application/json');
Expand Down
4 changes: 3 additions & 1 deletion packages/metro/src/lib/getAppendScripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import type {Dependency} from '../DeltaBundler/types.flow';
import CountingSet from './CountingSet';

const getInlineSourceMappingURL = require('../DeltaBundler/Serializers/helpers/getInlineSourceMappingURL');
const sourceMapString = require('../DeltaBundler/Serializers/sourceMapString');
const {
sourceMapString,
} = require('../DeltaBundler/Serializers/sourceMapString');
const countLines = require('./countLines');
const nullthrows = require('nullthrows');

Expand Down

0 comments on commit 34148e6

Please sign in to comment.