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

Support concurrent swc_common via feature #250

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support concurrent swc_common via feature #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ typescript = ["transforms", "swc_ecma_transforms_typescript"]
utils = ["swc_ecma_utils"]
view = ["dprint-swc-ext/view"]
visit = ["swc_ecma_visit", "swc_visit", "swc_visit_macros", "swc_macros_common"]
concurrent = ["swc_common/concurrent"]
Copy link
Member

@dsherret dsherret May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

As mentioned in #219 (comment) , I'm not sure this is a scenario we want to bother to support.

Do you really need this feature? How much faster have you found it? We've just started using separate threads per file. Maybe in bundling scenarios this is faster?

Copy link
Author

@spence spence May 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

unfortunately, a number of swc libraries bake-in swc_common's concurrent feature and including any of them prevents deno_ast from compiling:

i'm working from a fork of farm, which uses rayon for concurrent builds, and they benchmark against other bundlers using swc (e.g., rspack).

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. We can add a concurrent feature here, but no guarantee it's going to work in the future. It might be removed at any time due to the maintenance cost and you might have to contribute fixes to get it working again.


[dependencies]
anyhow = { version = "1.0.64", optional = true }
Expand Down
13 changes: 6 additions & 7 deletions src/source_map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,35 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use std::rc::Rc;

use crate::swc::common::sync::Lrc;
use crate::swc::common::SourceFile;

use crate::ModuleSpecifier;

#[derive(Clone, Default)]
pub struct SourceMap {
inner: Rc<crate::swc::common::SourceMap>,
inner: Lrc<crate::swc::common::SourceMap>,
}

impl SourceMap {
pub fn single(specifier: ModuleSpecifier, source: String) -> Self {
let map = Self::default();
map
.inner
.new_source_file(Rc::new(swc_common::FileName::Url(specifier)), source);
.new_source_file(Lrc::new(swc_common::FileName::Url(specifier)), source);
map
}

pub fn inner(&self) -> &Rc<crate::swc::common::SourceMap> {
pub fn inner(&self) -> &Lrc<crate::swc::common::SourceMap> {
&self.inner
}

pub fn new_source_file(
&self,
specifier: ModuleSpecifier,
source: String,
) -> Rc<SourceFile> {
) -> Lrc<SourceFile> {
self
.inner
.new_source_file(Rc::new(swc_common::FileName::Url(specifier)), source)
.new_source_file(Lrc::new(swc_common::FileName::Url(specifier)), source)
}
}
7 changes: 5 additions & 2 deletions src/transpiling/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use std::borrow::Cow;
use std::rc::Rc;
use std::sync::Arc;

use anyhow::Result;
Expand All @@ -14,6 +13,7 @@ use crate::swc::ast::Program;
use crate::swc::common::chain;
use crate::swc::common::comments::SingleThreadedComments;
use crate::swc::common::errors::Diagnostic as SwcDiagnostic;
use crate::swc::common::sync::{Lrc, Send, Sync};
use crate::swc::parser::error::SyntaxError;
use crate::swc::transforms::fixer;
use crate::swc::transforms::helpers;
Expand Down Expand Up @@ -349,9 +349,12 @@ fn transpile(

#[derive(Default, Clone)]
struct DiagnosticCollector {
diagnostics_cell: Rc<RefCell<Vec<SwcDiagnostic>>>,
diagnostics_cell: Lrc<RefCell<Vec<SwcDiagnostic>>>,
}

unsafe impl Send for DiagnosticCollector {}
unsafe impl Sync for DiagnosticCollector {}
Copy link
Author

Choose a reason for hiding this comment

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

we can feature gate these, but without concurrent, Send and Sync are noops:

#[cfg(not(feature = "concurrent"))]
mod single {
    pub use once_cell::unsync::{Lazy, OnceCell};
    /// Dummy trait because swc_common is in single thread mode.
    pub trait Send {}
    /// Dummy trait because swc_common is in single thread mode.
    pub trait Sync {}

    impl<T> Send for T where T: ?Sized {}
    impl<T> Sync for T where T: ?Sized {}

    pub use std::{
        cell::{
            Ref as ReadGuard, RefMut as WriteGuard, RefMut as MappedWriteGuard,
            RefMut as LockGuard, RefMut as MappedLockGuard,
        },
        rc::{Rc as Lrc, Weak},
    };
}

Copy link
Member

Choose a reason for hiding this comment

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

If someone is using concurrent then this not being thread safe will cause thread concurrency issues. Probably this will need a thread safe implementation when compiling with concurrent.


impl DiagnosticCollector {
pub fn into_handler(self) -> crate::swc::common::errors::Handler {
crate::swc::common::errors::Handler::with_emitter(
Expand Down
10 changes: 5 additions & 5 deletions src/transpiling/transforms.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ mod test {
use crate::swc::ast::Module;
use crate::swc::codegen::text_writer::JsWriter;
use crate::swc::codegen::Node;
use crate::swc::common::sync::Lrc;
use crate::swc::common::FileName;
use crate::swc::common::SourceMap;
use crate::swc::parser::Parser;
Expand All @@ -298,7 +299,6 @@ mod test {
use crate::swc::visit::FoldWith;
use crate::ModuleSpecifier;
use pretty_assertions::assert_eq;
use std::rc::Rc;

use super::*;

Expand Down Expand Up @@ -498,10 +498,10 @@ mod test {
assert_eq!(output, format!("{}\n", expected_output));
}

fn parse(src: &str) -> (Rc<SourceMap>, Module) {
let source_map = Rc::new(SourceMap::default());
fn parse(src: &str) -> (Lrc<SourceMap>, Module) {
let source_map = Lrc::new(SourceMap::default());
let source_file = source_map.new_source_file(
Rc::new(FileName::Url(
Lrc::new(FileName::Url(
ModuleSpecifier::parse("file:///test.ts").unwrap(),
)),
src.to_string(),
Expand All @@ -514,7 +514,7 @@ mod test {
(source_map, parser.parse_module().unwrap())
}

fn print(source_map: Rc<SourceMap>, module: Module) -> String {
fn print(source_map: Lrc<SourceMap>, module: Module) -> String {
let mut buf = vec![];
{
let mut writer =
Expand Down