Switch to GenMapping for sourcemap generation #1190

jridgewell · 2022-04-30T22:44:17Z

gen-mapping is faster, smaller, and lighter sourcemap generation library. This doesn't remove WASM (because SourceMapGenerator doesn't require it), but it's still nice to get off an unmaintained dependency.

This also exposes a new decodedMap property on the result object. Decoded maps are free to create (it's a shallow clone of the GenMapping instance), and passing them to @jridgewell/trace-mapping is copy-free. With Babel recently adding a decodedMap field, a dev could pass from the Babel transpilation to Terser without any added memory use for sourcemaps.

And if there's a multi stage build process, a dev could use @ampproject/remapping to remap Babel, Terser, and (eg) a bundler's outputs without having to feed input maps into each stage.

[`gen-mapping`](https://github.com/jridgewell/gen-mapping) is faster, smaller, and lighter sourcemap generation library. This also exposes a new `decodedMap` property on the result object. Decoded maps are free to create (it's a shallow clone of the `GenMapping` instance), and passing them to `@jridgewell/trace-mapping` is copy-free. With Babel [recently](babel/babel#14497) adding a `decodedMap` field, a dev could pass from the Babel transpilation to Terser without any added memory use for sourcemaps. And if there's a multi stage build process, a dev could use `@ampproject/remapping` to remap Babel, Terser, and (eg) a bundler's outputs without having to feed input maps into each stage.

jridgewell · 2022-04-30T22:46:50Z

lib/sourcemap.js

-        generator.addMapping({
-            generated : { line: gen_line + options.dest_line_diff, column: gen_col },
-            original  : { line: orig_line + options.orig_line_diff, column: orig_col },
+        maybeAddMapping(generator, {


maybeAddMapping produces a cleaner sourcemap by discarding mappings that add no value. Eg, if two consecutive segments point to the same source location, that provides no new information.

This is a change from the source-map's addMapping. If you'd like, I can switch to gen-mapping's addMapping, which gives us the same keep-everything behavior.

Sounds like less RAM usage to me, so this is a nice change!

jridgewell · 2022-04-30T22:47:39Z

lib/minify.js

+                    });
+                }
+            });
+            result.decodedMap = options.format.source_map.getDecoded();


Decoded maps are free to generate from GenMapping (because mappings are stored in decoded form), it's just a shallow clone.

jridgewell · 2022-04-30T22:48:44Z

lib/minify.js

+                enumerable: true,
+                get() {
+                    const map = options.format.source_map.getEncoded();
+                    return (result.map = options.sourceMap.asObject ? map : JSON.stringify(map));


Encoded maps require serialization from decoded form into an encoded VLQ string. I've placed this behind a getter so that it's not performed unless required by the dev.

Unrelated suggestion:

I looked at the implementation underneath getEncoded here and I wonder if a JSON string rendition of it could be optimized.

VLQ strings are guaranteed to not contain \ or " so for large maps it may be more memory efficient to generate the JSON manually.

'{(other sourcemap properties...), "mappings": "' + encode(decoded.mappings) + '"}'

I'm making the assumption that V8 would make this more memory efficient by not copying the encoded string into the JSON, but instead making the output JSON into a rope containing 3 strings.

jridgewell · 2022-04-30T22:52:22Z

lib/minify.js

            });
-            if (options.sourceMap.includeSources) {


This section was moved into the SourceMap. Before, calling setSourceContent for the original source contents and the input contents would mean there are 2 contents stored because they have different file names. SourceMapGenerator would traverse all mappings and determine if a particular file isn't referenced in the mappings, and would exclude that file's contents from the output.

GenMapping doesn't do that traversal. If you tell it that file.js has contents, then file.js will be in the output map even if it's not actually referenced by any mappings. By moving this into SourceMap, we can do a property lookup for source contents as they're needed.

fabiosantoscode

Requested a couple of changes.

Additionally, can you update the typescript definition? It still refers to source-map and doesn't have the new decoded_map result property.

https://github.com/terser/terser/blob/master/tools/terser.d.ts

lib/minify.js

fabiosantoscode · 2022-05-01T11:25:39Z

package.json

    "@jridgewell/trace-mapping": "^0.3.5",
    "acorn": "^8.5.0",
    "commander": "^2.20.0",
-    "source-map": "~0.8.0-beta.0",


The tests still use this (namely test/mocha/sourcemaps.js) so it should go into devDependencies

Ah, I didn't purge my node_modules. Fixed.

fabiosantoscode · 2022-05-01T11:36:04Z

I'm thinking both this PR and your previous #1180 are breaking changes, as they change how Terser can be loaded in the browser without a package manager (see https://github.com/terser/terser#api-reference).

So the next release will be Terser 6.0.0, and I'll likely take away this browser-loading capability.

Co-authored-by: Fábio Santos <fabiosantosart@gmail.com>

jridgewell

as they change how Terser can be loaded in the browser without a package manager

Oh, we forgot to update the rollup.config.js file
I wrote a small wrapper package that will provides the same SourceMapConsumer and SourceMapGenerator interface as source-map. That way, if loaded in node, it'll require the new libraries. And if a loaded in a browser, the dev could load either source-map or @jridgewell/source-map and it'll work.

This preserves API compatibility, so that loading source-map in the browser is still supported.

fabiosantoscode · 2022-05-01T22:49:57Z

I've noticed a small uptick in memory usage, but better speed.

I'm comparing with this branch merged to master, not just this branch.

## Before
--before creating OutputStream and printing 
heapTotal 790.9 MB                          
heapUsed 571.83 MB                          
--after OutputStream prints                 
heapTotal 967.02 MB                         
heapUsed 698.39 MB                          
--after assigning decoded_map
heapTotal 950.77 MB                         
heapUsed 707.11 MB                          
--after result.map getter
heapTotal 935.77 MB                         
heapUsed 707.11 MB

## After
--before creating OutputStream and printing
heapTotal 791.15 MB                        
heapUsed 571.63 MB                         
--after OutputStream prints                
heapTotal 1072.77 MB                       
heapUsed 713.45 MB                         
--after assigning decoded_map              
heapTotal 1024.52 MB                       
heapUsed 712.96 MB                         
--after result.map getter                  
heapTotal 990.77 MB                        
heapUsed 730.88 MB

This isn't concerning at all, just wanted to point it out.

fabiosantoscode · 2022-05-02T00:03:38Z

Since you're switching to a library called source-map this might just be compatible with the current browser usage!

fabiosantoscode · 2022-05-02T00:04:09Z

I'll check later. Sadly I'm back from holidays tomorrow so I'll be as non-responsive as usual.

jridgewell · 2022-05-02T00:30:40Z

I've noticed a small uptick in memory usage, but better speed.

This is a little surprising. I added memory usage benchmarks which are favorable. I can massage the addMapping call a bit to avoid temporary object allocations. The generated and original objects, and the mapping object itself are all temporary in both APIs. We could allocate once, assign properties, and call.

Since you're switching to a library called source-map this might just be compatible with the current browser usage!

Hopefully. I reused the same window.sourceMap name in UMD, and provide the same class and prototype methods.

fabiosantoscode · 2022-05-07T19:24:16Z

lib/sourcemap.js

+        // We support both @jridgewell/source-map (which has a sync
+        // SourceMapConsumer) and source-map (which has an async
+        // SourceMapConsumer).
+        orig_map = await new SourceMapConsumer(options.orig);


tools/terser.d.ts

fabiosantoscode · 2022-05-08T11:12:08Z

This looks good to merge!

fabiosantoscode · 2022-05-10T01:56:59Z

Awesome work, thanks @jridgewell!

The `decodedMap` prop comes from terser/terser#1190 > This also exposes a new `decodedMap` property on the result object. Decoded maps are free to create (it's a shallow clone of the `GenMapping` instance), and passing them to `@jridgewell/trace-mapping` is copy-free. With Babel [recently](babel/babel#14497) adding a `decodedMap` field, a dev could pass from the Babel transpilation to Terser without any added memory use for sourcemaps.

jridgewell force-pushed the gen-mapping branch from d3d79d3 to d23354e Compare April 30, 2022 22:45

jridgewell commented Apr 30, 2022

View reviewed changes

fabiosantoscode requested changes May 1, 2022

View reviewed changes

jridgewell and others added 2 commits May 1, 2022 10:47

Apply suggestions from code review

04953b4

Co-authored-by: Fábio Santos <fabiosantosart@gmail.com>

Fix dependency

8f938df

jridgewell force-pushed the gen-mapping branch from 2273542 to 970a8c9 Compare May 1, 2022 20:54

jridgewell commented May 1, 2022

View reviewed changes

jridgewell added 2 commits May 1, 2022 17:09

Switch to source-map compatible @jridgewell/source-map

f6dbb8f

This preserves API compatibility, so that loading source-map in the browser is still supported.

Update TS types

b2e5a12

jridgewell force-pushed the gen-mapping branch from 970a8c9 to b2e5a12 Compare May 1, 2022 21:09

Support source-map's async SourceMapConsumer

6cab7f2

fabiosantoscode reviewed May 7, 2022

View reviewed changes

tools/terser.d.ts Show resolved Hide resolved

Fix sourcesContent check with source-map's SourceMapGenerator

2cc6270

jridgewell mentioned this pull request May 9, 2022

wg-performance Status Update 2022-05-09 ampproject/wg-performance#112

Closed

fabiosantoscode merged commit a47f29a into terser:master May 10, 2022

jridgewell deleted the gen-mapping branch May 10, 2022 01:59

boris-petrov mentioned this pull request May 31, 2022

Test against Node v18 ember-cli/ember-cli#9905

Closed

S11001001 mentioned this pull request Aug 1, 2022

Address navigator frontend security advisories digital-asset/daml#14583

Merged

tanner-reits mentioned this pull request Oct 10, 2022

chore(deps-dev): bump terser from 5.6.1 to 5.15.1 ionic-team/stencil#3687

Merged

CvX mentioned this pull request Nov 19, 2022

DEV: Update terser discourse/discourse#19109

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to GenMapping for sourcemap generation #1190

Switch to GenMapping for sourcemap generation #1190

jridgewell commented Apr 30, 2022

jridgewell Apr 30, 2022

fabiosantoscode May 1, 2022

jridgewell Apr 30, 2022

jridgewell Apr 30, 2022

fabiosantoscode May 1, 2022 •

edited

jridgewell Apr 30, 2022

fabiosantoscode left a comment

fabiosantoscode May 1, 2022

jridgewell May 1, 2022

fabiosantoscode commented May 1, 2022

jridgewell left a comment

fabiosantoscode commented May 1, 2022

fabiosantoscode commented May 2, 2022

fabiosantoscode commented May 2, 2022

jridgewell commented May 2, 2022

fabiosantoscode May 7, 2022

fabiosantoscode commented May 8, 2022

fabiosantoscode commented May 10, 2022

Switch to GenMapping for sourcemap generation #1190

Switch to GenMapping for sourcemap generation #1190

Conversation

jridgewell commented Apr 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiosantoscode May 1, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiosantoscode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiosantoscode commented May 1, 2022

jridgewell left a comment

Choose a reason for hiding this comment

fabiosantoscode commented May 1, 2022

fabiosantoscode commented May 2, 2022

fabiosantoscode commented May 2, 2022

jridgewell commented May 2, 2022

Choose a reason for hiding this comment

fabiosantoscode commented May 8, 2022

fabiosantoscode commented May 10, 2022

fabiosantoscode May 1, 2022 •

edited