Improve performance by directly constructing AST from buffer (#5391)

* Use consistent module resolution

* Fix number literal casing

* Store esTreeNode of import expression argument on ImportExpression

* Move ReadString type

* Pre-generate keys for iteration

* Separate parsing and node construction

* Add missing AST nodes and improve some names

* Add additional Node keys and adapt some logic

This prepares for the new buffer parsing

* Initial version of bufferParsers

TODO: Special logic in parseNode of some nodes

* Use absolute positions

This should make future template logic easier

* Move annotation flags to the annotated nodes

* Move invalid annotation handling to Program

* Improve expression

* Make parse and panic errors proper nodes

* Directly consume buffer

We still need to update generate-buffer-parsers
with the provided changes.

* Provide correct scopes to nodes

* Add post-processing steps

* Move static flag to first position

* Add remaining customizations

* Create new perf script that measures against node_modules rollup

* Run form tests twice to improve coverage

The second run uses the cache

* Avoid unnecessary cache generation

* Do not generate cache on CLI unless requested
* Keep AST if cache is not explicitly disabled

* Test panic errors for synchronous parsing

* Do not run the leak test locally on npm test

This does not work on non-x86 architectures

* Test custom AST

* Remove unnecessary conditional

* Add descriptions for all relevant test categories

Author: lukastaegert

.eslintrc.js

@@ -151,6 +151,7 @@ module.exports = {
 		'unicorn/no-null': 'off',
 		'unicorn/no-this-assignment': 'off',
 		'unicorn/no-useless-undefined': 'off',
+		'unicorn/number-literal-case': 'off',
 		'unicorn/prefer-at': 'off',
 		'unicorn/prefer-code-point': 'off',
 		'unicorn/prefer-math-trunc': 'off',

CONTRIBUTING.md

@@ -56,6 +56,32 @@ npm run test:quick
 
 Note that this does not run the browser tests and a few CLI tests will fail.
 
+### How to write tests
+
+For any new feature or bug fix, sufficient test coverage is crucial.
+
+Note that Rollup does not really have unit tests, only the external APIs are tested with the full Rollup build. While this may seem unusual, the tests are still very stable and fast. This provides us with the ability to perform major refactorings of the code base while ensuring full compatibility with the previous versions.
+
+There are different test categories. Most of these tests are directory-based where you have a directory with a `_config.js` file that contains the test description and configuration and several code files. See [/test/types.d.ts](./test/types.d.ts) for a full list of available test configuration options for all directory based test types. By default, unless specified otherwise, the `main.js` file is the entry point for the test. To run the tests in an IDE, configure a ["Mocha" compatible test runner](https://mochajs.org/#editor-plugins) that uses `test/test.js` as the entry point.
+
+- **[`test/function`](./test/function/samples)**: These tests bundle to CommonJS and then run the entry point provided by `main.js`. The `assert` function from `node:assert` is injected as a global variable, so you can make inline assertions in the code. You can also use the `exports` configuration key to make assertions on the exported values. These are very stable and meaningful tests and should be your first choice for new tests.
+  - For regression testing when Rollup produces invalid code or crashes
+  - For testing plugin interactions. To do so, import `node:assert` in your `_config.js` file and make assertions in your plugin hooks as needed.
+  - For testing expected bundling errors, warnings and logs (use the `error`, `generateError`, `warnings` and `logs` configuration keys)
+  - For asserting on the generated bundle object (use the `bundle` configuration key)
+- **[`test/form`](./test/form/samples)**: These tests bundle to all output formats and do not run the code. They compare the bundled code against an `_expected` directory that contains the output for all formats. If the format is not important, you can specify an `_expected.js` file instead, which will be compared against the output when bundling to ES module format.
+  - For testing tree-shaking
+  - For testing code that does not run on all supported NodeJS platforms
+- **[`test/chunking-form`](./test/chunking-form/samples)**: Similar to the `form` tests, these tests support multiple output files and assets. Instead of a single file, there is a directory for each output format.
+- **[`test/cli`](./test/cli/samples)**: These tests run the Rollup CLI with a given configuration. They can compare the generated files against provided files and make assertions on stderr output. They can also optionally run the generated files.
+- **[`test/watch`](./test/watch)**: Test that watch mode works as expected. These tests are actually in the `index.js` file and only use the `samples` directory for input files.
+- **[`test/browser`](./test/browser/samples)**: These tests bundle with the browser build of Rollup. They compare the output to an `_expected` directory and allow to make assertions on bundling errors. Note that you need to provide all input files via plugins.
+- **[`test/sourcemaps`](./test/sourcemaps/samples)**: Tests to make assertions on the generated sourcemaps.
+- **[`test/incremental`](./test/incremental)**: For testing the caching behaviour of Rollup. As these tests need to run Rollup more than once, it was not easily possible to implement them as directory-based tests.
+- **[`test/file-hashes`](./test/file-hashes/samples)**: Relevant for testing that different outputs have different file hashes. With the new hashing algorithm, these tests are not as important as they used to be and are kept mostly for historical reasons.
+- **[`test/hooks`](./test/hooks)**: Do not add new tests here. These tests were the original tests for the plugin interface. For new tests, `function` tests are preferred as they are much easier to maintain.
+- **[`test/misc`](./test/misc)**: General tests that do not fit into the other categories.
+
 ### Developing with the website
 
 Running

cli/run/index.ts

@@ -61,6 +61,12 @@ export default async function runRollup(command: Record<string, any>): Promise<v
 			const { options, warnings } = await getConfigs(command);
 			try {
 				for (const inputOptions of options) {
+					if (!inputOptions.cache) {
+						// We explicitly disable the cache when unused as the CLI will not
+						// use the cache object on the bundle when not in watch mode. This
+						// improves performance as the cache is not generated.
+						inputOptions.cache = false;
+					}
 					await build(inputOptions, warnings, command.silent);
 				}
 				if (command.failAfterWarnings && warnings.warningOccurred) {

package.json

@@ -63,8 +63,7 @@
     "lint:markdown:nofix": "prettier --check \"**/*.md\"",
     "lint:rust": "cd rust && cargo fmt && cargo clippy --fix --allow-dirty",
     "lint:rust:nofix": "cd rust && cargo fmt --check && cargo clippy",
-    "perf": "npm run build:cjs && node --expose-gc scripts/perf.js",
-    "perf:init": "node scripts/perf-init.js",
+    "perf": "npm run build && node --expose-gc scripts/perf.js",
     "prepare": "husky && node scripts/check-release.js || npm run build:prepare",
     "prepublishOnly": "node scripts/check-release.js && node scripts/prepublish.js",
     "postpublish": "node scripts/postpublish.js",
@@ -75,7 +74,7 @@
     "test:update-snapshots": "node scripts/update-snapshots.js",
     "test:cjs": "npm run build:cjs && npm run test:only",
     "test:quick": "mocha -b test/test.js",
-    "test:all": "concurrently --kill-others-on-fail -c green,blue,magenta,cyan,red 'npm:test:only' 'npm:test:browser' 'npm:test:typescript' 'npm:test:leak' 'npm:test:package' 'npm:test:options'",
+    "test:all": "concurrently --kill-others-on-fail -c green,blue,magenta,cyan,red 'npm:test:only' 'npm:test:browser' 'npm:test:typescript' 'npm:test:package' 'npm:test:options'",
     "test:coverage": "npm run build:cjs && shx rm -rf coverage/* && nyc --reporter html mocha test/test.js",
     "test:coverage:browser": "npm run build && shx rm -rf coverage/* && nyc mocha test/browser/index.js",
     "test:leak": "node --expose-gc test/leak/index.js",

rust/parse_ast/src/convert_ast/converter.rs

@@ -2397,7 +2397,7 @@ impl<'a> AstConverter<'a> {
     self.add_explicit_end(end_position, self.code.len() as u32);
     // annotations, these need to come after end so that trailing comments are
     // included
-    self.update_reference_position(end_position + PROGRAM_ANNOTATIONS_OFFSET);
+    self.update_reference_position(end_position + PROGRAM_INVALID_ANNOTATIONS_OFFSET);
     self.index_converter.invalidate_collected_annotations();
     let invalid_annotations = self.index_converter.take_invalid_annotations();
     self.convert_item_list(&invalid_annotations, |ast_converter, annotation| {

rust/parse_ast/src/convert_ast/converter/ast_constants.rs

@@ -1,8 +1,8 @@
 // This file is generated by scripts/generate-ast-converters.js.
 // Do not edit this file directly.
 
-pub const TYPE_PARSE_ERROR: [u8; 4] = 0u32.to_ne_bytes();
-pub const TYPE_PANIC_ERROR: [u8; 4] = 1u32.to_ne_bytes();
+pub const TYPE_PANIC_ERROR_INLINED_MESSAGE: [u8; 4] = 0u32.to_ne_bytes();
+pub const TYPE_PARSE_ERROR_INLINED_MESSAGE: [u8; 4] = 1u32.to_ne_bytes();
 pub const TYPE_ARRAY_EXPRESSION_INLINED_ELEMENTS: [u8; 4] = 2u32.to_ne_bytes();
 pub const TYPE_ARRAY_PATTERN_INLINED_ELEMENTS: [u8; 4] = 3u32.to_ne_bytes();
 pub const TYPE_ARROW_FUNCTION_EXPRESSION_INLINED_ANNOTATIONS: [u8; 4] = 4u32.to_ne_bytes();
@@ -81,6 +81,10 @@ pub const TYPE_VARIABLE_DECLARATOR_INLINED_ID: [u8; 4] = 76u32.to_ne_bytes();
 pub const TYPE_WHILE_STATEMENT_INLINED_TEST: [u8; 4] = 77u32.to_ne_bytes();
 pub const TYPE_YIELD_EXPRESSION: [u8; 4] = 78u32.to_ne_bytes();
 
+pub const PANIC_ERROR_RESERVED_BYTES: usize = 4;
+
+pub const PARSE_ERROR_RESERVED_BYTES: usize = 4;
+
 pub const ARRAY_EXPRESSION_RESERVED_BYTES: usize = 4;
 
 pub const ARRAY_PATTERN_RESERVED_BYTES: usize = 4;
@@ -245,8 +249,8 @@ pub const META_PROPERTY_PROPERTY_OFFSET: usize = 4;
 
 pub const METHOD_DEFINITION_RESERVED_BYTES: usize = 16;
 pub const METHOD_DEFINITION_FLAGS_OFFSET: usize = 4;
-pub const METHOD_DEFINITION_COMPUTED_FLAG: u32 = 1;
-pub const METHOD_DEFINITION_STATIC_FLAG: u32 = 2;
+pub const METHOD_DEFINITION_STATIC_FLAG: u32 = 1;
+pub const METHOD_DEFINITION_COMPUTED_FLAG: u32 = 2;
 pub const METHOD_DEFINITION_VALUE_OFFSET: usize = 8;
 pub const METHOD_DEFINITION_KIND_OFFSET: usize = 12;
 
@@ -261,7 +265,7 @@ pub const OBJECT_PATTERN_RESERVED_BYTES: usize = 4;
 pub const PRIVATE_IDENTIFIER_RESERVED_BYTES: usize = 4;
 
 pub const PROGRAM_RESERVED_BYTES: usize = 8;
-pub const PROGRAM_ANNOTATIONS_OFFSET: usize = 4;
+pub const PROGRAM_INVALID_ANNOTATIONS_OFFSET: usize = 4;
 
 pub const PROPERTY_RESERVED_BYTES: usize = 20;
 pub const PROPERTY_FLAGS_OFFSET: usize = 4;
@@ -274,8 +278,8 @@ pub const PROPERTY_KIND_OFFSET: usize = 16;
 
 pub const PROPERTY_DEFINITION_RESERVED_BYTES: usize = 12;
 pub const PROPERTY_DEFINITION_FLAGS_OFFSET: usize = 4;
-pub const PROPERTY_DEFINITION_COMPUTED_FLAG: u32 = 1;
-pub const PROPERTY_DEFINITION_STATIC_FLAG: u32 = 2;
+pub const PROPERTY_DEFINITION_STATIC_FLAG: u32 = 1;
+pub const PROPERTY_DEFINITION_COMPUTED_FLAG: u32 = 2;
 pub const PROPERTY_DEFINITION_VALUE_OFFSET: usize = 8;
 
 pub const REST_ELEMENT_RESERVED_BYTES: usize = 4;

rust/parse_ast/src/error_emit.rs

@@ -5,7 +5,10 @@ use parking_lot::Mutex;
 use swc_common::errors::{DiagnosticBuilder, Emitter, Handler, Level, HANDLER};
 use swc_ecma_ast::Program;
 
-use crate::convert_ast::converter::{ast_constants::TYPE_PARSE_ERROR, convert_string};
+use crate::convert_ast::converter::{
+  ast_constants::{PARSE_ERROR_RESERVED_BYTES, TYPE_PARSE_ERROR_INLINED_MESSAGE},
+  convert_string,
+};
 
 #[derive(Clone, Default)]
 struct Writer(Arc<Mutex<Vec<u8>>>);
@@ -65,9 +68,9 @@ where
 }
 
 fn create_error_buffer(wr: &Writer, code: &str) -> Vec<u8> {
-  let mut buffer = TYPE_PARSE_ERROR.to_vec();
+  let mut buffer = TYPE_PARSE_ERROR_INLINED_MESSAGE.to_vec();
   let mut lock = wr.0.lock();
-  let mut error_buffer = take(&mut *lock);
+  let error_buffer = take(&mut *lock);
   let pos = u32::from_ne_bytes(error_buffer[0..4].try_into().unwrap());
   let mut utf_16_pos: u32 = 0;
   for (utf_8_pos, char) in code.char_indices() {
@@ -76,7 +79,11 @@ fn create_error_buffer(wr: &Writer, code: &str) -> Vec<u8> {
     }
     utf_16_pos += char.len_utf16() as u32;
   }
-  error_buffer[0..4].copy_from_slice(&utf_16_pos.to_ne_bytes());
-  buffer.extend_from_slice(&error_buffer);
+  // start
+  buffer.extend_from_slice(&utf_16_pos.to_ne_bytes());
+  // end
+  buffer.resize(buffer.len() + PARSE_ERROR_RESERVED_BYTES, 0);
+  // message
+  buffer.extend_from_slice(&error_buffer[4..]);
   buffer
 }

rust/parse_ast/src/lib.rs

@@ -1,6 +1,8 @@
 use std::panic::{catch_unwind, AssertUnwindSafe};
 
-use convert_ast::converter::ast_constants::TYPE_PANIC_ERROR;
+use convert_ast::converter::ast_constants::{
+  PANIC_ERROR_RESERVED_BYTES, TYPE_PANIC_ERROR_INLINED_MESSAGE,
+};
 use convert_ast::converter::{convert_string, AstConverter};
 use swc_common::sync::Lrc;
 use swc_common::{FileName, FilePathMapping, Globals, SourceMap, GLOBALS};
@@ -52,20 +54,19 @@ pub fn parse_ast(code: String, allow_return_outside_function: bool) -> Vec<u8> {
         }
       }
     }));
-    match result {
-      Ok(buffer) => buffer,
-      Err(err) => {
-        let msg = if let Some(msg) = err.downcast_ref::<&str>() {
-          msg
-        } else if let Some(msg) = err.downcast_ref::<String>() {
-          msg
-        } else {
-          "Unknown rust panic message"
-        };
-        let mut buffer = TYPE_PANIC_ERROR.to_vec();
-        convert_string(&mut buffer, msg);
-        buffer
-      }
-    }
+    result.unwrap_or_else(|err| {
+      let msg = if let Some(msg) = err.downcast_ref::<&str>() {
+        msg
+      } else if let Some(msg) = err.downcast_ref::<String>() {
+        msg
+      } else {
+        "Unknown rust panic message"
+      };
+      let mut buffer = TYPE_PANIC_ERROR_INLINED_MESSAGE.to_vec();
+      // reserve for start and end even though they are unused
+      buffer.resize(buffer.len() + 4 + PANIC_ERROR_RESERVED_BYTES, 0);
+      convert_string(&mut buffer, msg);
+      buffer
+    })
   })
 }