Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
122 changes: 50 additions & 72 deletions src/bundler/LinkerContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2462,7 +2462,7 @@ impl<'a> LinkerContext<'a> {
pub fn append_isolated_hashes_for_imported_chunks(
&self,
hash: &mut ContentHasher,
chunks: &mut [Chunk],
chunks: &[Chunk],
index: u32,
chunk_visit_map: &mut AutoBitSet,
) {
Expand All @@ -2475,19 +2475,14 @@ impl<'a> LinkerContext<'a> {
}
chunk_visit_map.set(index as usize);

let chunk = &chunks[index as usize];

// Visit the other chunks that this chunk imports before visiting this chunk
// Note: reshaped for borrowck — collect imports first to avoid aliasing &chunks[index] with recursive &mut chunks
let cross_chunk_imports: Vec<u32> = chunks[index as usize]
.cross_chunk_imports
.slice()
.iter()
.map(|import| import.chunk_index)
.collect();
for chunk_index in cross_chunk_imports {
for import in chunk.cross_chunk_imports.slice() {
self.append_isolated_hashes_for_imported_chunks(
hash,
chunks,
chunk_index,
import.chunk_index,
chunk_visit_map,
);
}
Expand All @@ -2496,76 +2491,59 @@ impl<'a> LinkerContext<'a> {
// express cross-chunk dependencies via `cross_chunk_imports` above, but
// HTML (and CSS) chunks only reference other chunks through pieces, so
// recurse on those too.
// Note: reshaped for borrowck — collect piece queries first so the
// `&chunks[index]` borrow is dropped before the recursive `&mut chunks`
// calls in the Chunk/Scb arms below. `final_rel_path` is re-indexed per
// Asset arm (not hoisted) because it is now `Box<[u8]>` (not `Copy`).
let piece_queries: Vec<(crate::chunk::QueryKind, u32)> =
if let crate::chunk::IntermediateOutput::Pieces(pieces) =
&chunks[index as usize].intermediate_output
{
pieces
.slice()
.iter()
.map(|p| (p.query.kind(), p.query.index()))
.collect()
} else {
Vec::new()
};

for (kind, piece_index) in piece_queries {
match kind {
crate::chunk::QueryKind::Asset => {
let mut from_chunk_dir = bun_paths::resolve_path::dirname::<
bun_paths::resolve_path::platform::Posix,
>(
&chunks[index as usize].final_rel_path
);
if from_chunk_dir == b"." {
from_chunk_dir = b"";
}
if let crate::chunk::IntermediateOutput::Pieces(pieces) = &chunk.intermediate_output {
for p in pieces.slice() {
match p.query.kind() {
crate::chunk::QueryKind::Asset => {
let mut from_chunk_dir = bun_paths::resolve_path::dirname::<
bun_paths::resolve_path::platform::Posix,
>(&chunk.final_rel_path);
if from_chunk_dir == b"." {
from_chunk_dir = b"";
}

let source_index = piece_index;
let parse_graph = self.parse_graph();
let additional_files: &[AdditionalFile] =
parse_graph.input_files.items_additional_files()[source_index as usize]
.slice();
debug_assert!(!additional_files.is_empty());
match &additional_files[0] {
AdditionalFile::OutputFile(output_file_id) => {
let path = &parse_graph.additional_output_files
[*output_file_id as usize]
.dest_path;
hash.write(bun_paths::resolve_path::relative_platform::<
bun_paths::resolve_path::platform::Posix,
false,
>(from_chunk_dir, path));
let source_index = p.query.index();
let parse_graph = self.parse_graph();
let additional_files: &[AdditionalFile] =
parse_graph.input_files.items_additional_files()[source_index as usize]
.slice();
debug_assert!(!additional_files.is_empty());
match &additional_files[0] {
AdditionalFile::OutputFile(output_file_id) => {
let path = &parse_graph.additional_output_files
[*output_file_id as usize]
.dest_path;
hash.write(bun_paths::resolve_path::relative_platform::<
bun_paths::resolve_path::platform::Posix,
false,
>(from_chunk_dir, path));
}
AdditionalFile::SourceIndex(_) => {}
}
AdditionalFile::SourceIndex(_) => {}
}
crate::chunk::QueryKind::Chunk => {
self.append_isolated_hashes_for_imported_chunks(
hash,
chunks,
p.query.index(),
chunk_visit_map,
);
}
crate::chunk::QueryKind::Scb => {
self.append_isolated_hashes_for_imported_chunks(
hash,
chunks,
self.graph.files.items_entry_point_chunk_index()
[p.query.index() as usize],
chunk_visit_map,
);
}
crate::chunk::QueryKind::None | crate::chunk::QueryKind::HtmlImport => {}
}
crate::chunk::QueryKind::Chunk => {
self.append_isolated_hashes_for_imported_chunks(
hash,
chunks,
piece_index,
chunk_visit_map,
);
}
crate::chunk::QueryKind::Scb => {
self.append_isolated_hashes_for_imported_chunks(
hash,
chunks,
self.graph.files.items_entry_point_chunk_index()[piece_index as usize],
chunk_visit_map,
);
}
crate::chunk::QueryKind::None | crate::chunk::QueryKind::HtmlImport => {}
}
}

// Mix in the hash for this chunk
let chunk = &chunks[index as usize];
hash.write(&chunk.isolated_hash.to_ne_bytes());
}

Expand Down
45 changes: 15 additions & 30 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2132,21 +2132,16 @@ pub mod bv2_impl {
) {
let file_map_result = _file_map_result;
let mut path_primary = file_map_result.path_pair.primary;
// reshaped for borrowck — `get_or_put` borrows `*self` mutably via
// `self.graph`; capture the slot as `*mut u32` so subsequent `self.*` calls
// type-check. SAFETY: `path_to_source_index_map(target)` is not mutated again
// until after the last `*value_ptr` access below.
let (found_existing, value_ptr): (bool, *mut u32) = {
let entry = self
.path_to_source_index_map(target)
.get_or_put(path_primary.text)
.expect("oom");
(
entry.found_existing,
std::ptr::from_mut::<u32>(entry.value_ptr),
)
};
if !found_existing {
if let Some(existing) =
self.path_to_source_index_map(target).get(path_primary.text)
{
let record: &mut ImportRecord =
&mut self.graph.ast.items_import_records_mut()
[import_record.importer_source_index as usize]
.as_mut_slice()
[import_record.import_record_index as usize];
record.source_index = Index::init(existing);
} else {
let loader: Loader = 'brk: {
let record: &mut ImportRecord =
&mut self.graph.ast.items_import_records_mut()
Expand Down Expand Up @@ -2176,22 +2171,15 @@ pub mod bv2_impl {
import_record.original_target,
)
.expect("oom");
// SAFETY: see `value_ptr` note above.
unsafe { *value_ptr = idx };
self.path_to_source_index_map(target)
.put(path_primary.text, idx)
.expect("oom");
let record: &mut ImportRecord =
&mut self.graph.ast.items_import_records_mut()
[import_record.importer_source_index as usize]
.as_mut_slice()
[import_record.import_record_index as usize];
record.source_index = Index::init(idx);
} else {
let record: &mut ImportRecord =
&mut self.graph.ast.items_import_records_mut()
[import_record.importer_source_index as usize]
.as_mut_slice()
[import_record.import_record_index as usize];
// SAFETY: see `value_ptr` note above.
record.source_index = Index::init(unsafe { *value_ptr });
}
return;
}
Expand Down Expand Up @@ -2437,9 +2425,6 @@ pub mod bv2_impl {
// For example, it is silly to bundle index.css depended on by client+server twice.
// It makes sense to separate these for JS because the target affects DCE
if self.transpiler.options.server_components && !loader.is_javascript_like() {
// reshaped for borrowck — cannot hold two `&mut` into
// `self.graph` simultaneously, so re-derive the map per insert.
let key_text: Box<[u8]> = path.text.to_vec().into_boxed_slice();
let main_target = self.transpiler.options.target;
let separate_ssr = self
.framework
Expand All @@ -2455,11 +2440,11 @@ pub mod bv2_impl {
_ => (Target::Browser, Target::ServerComponentsSsr),
};
self.path_to_source_index_map(ta)
.put(&key_text, idx)
.put(path.text, idx)
.expect("oom");
if separate_ssr {
self.path_to_source_index_map(tb)
.put(&key_text, idx)
.put(path.text, idx)
.expect("oom");
}
}
Expand Down
22 changes: 11 additions & 11 deletions src/bundler/linker_context/computeChunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,7 @@ pub fn compute_chunks(this: &mut LinkerContext, unique_key: u64) -> crate::Resul
for (entry_id_, &source_index) in entry_source_indices.iter().enumerate() {
let entry_bit = entry_id_ as chunk::EntryPointId;

// reshaped for borrowck — set the bit through a scoped &mut, then keep an
// owned clone so the `this.graph.files` borrow does not span the helper calls below
// that need `&LinkerContext` / `&mut LinkerContext`.
let entry_bits: AutoBitSet = {
let eb = &mut this.graph.files.items_entry_bits_mut()[source_index as usize];
eb.set(entry_bit as usize);
eb.clone()?
};
this.graph.files.items_entry_bits_mut()[source_index as usize].set(entry_bit as usize);

let has_html_chunk = loaders[source_index as usize] == Loader::Html;

Expand All @@ -114,7 +107,10 @@ pub fn compute_chunks(this: &mut LinkerContext, unique_key: u64) -> crate::Resul
// entry_bits is arbitrary bytes (not UTF-8) and cannot go through fmt::Display.
let mut v = bun_alloc::ArenaVec::new_in(temp);
v.push((!has_html_chunk) as u8);
v.extend_from_slice(entry_bits.bytes(this.graph.entry_points.len()));
v.extend_from_slice(
this.graph.files.items_entry_bits()[source_index as usize]
.bytes(this.graph.entry_points.len()),
);
break 'brk v.into_bump_slice();
}
};
Expand Down Expand Up @@ -150,7 +146,10 @@ pub fn compute_chunks(this: &mut LinkerContext, unique_key: u64) -> crate::Resul
// always generated even if the resulting file is empty
let hash_to_use = if !this.options.css_chunking {
bun_wyhash::hash(
temp.alloc_slice_copy(entry_bits.bytes(this.graph.entry_points.len())),
temp.alloc_slice_copy(
this.graph.files.items_entry_bits()[source_index as usize]
.bytes(this.graph.entry_points.len()),
),
)
} else {
let mut hasher = Wyhash::init(5);
Expand Down Expand Up @@ -257,7 +256,8 @@ pub fn compute_chunks(this: &mut LinkerContext, unique_key: u64) -> crate::Resul
}
*css_chunk_entry.value_ptr = Chunk {
entry_point: chunk::EntryPoint::new(source_index, entry_bit, true, false),
entry_bits: entry_bits.clone()?,
entry_bits: this.graph.files.items_entry_bits()[source_index as usize]
.clone()?,
content: chunk::Content::Css(chunk::CssChunk {
imports_in_chunk_in_order: order,
asts: (0..order_len)
Expand Down
7 changes: 3 additions & 4 deletions src/bundler/linker_context/computeCrossChunkDependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,8 @@ fn compute_cross_chunk_dependencies_with_chunk_metas(
}

// Find all uses in this chunk of symbols from other chunks
// reshaped for borrowck — collect keys first to avoid holding a borrow on
// chunk_metas[chunk_index] while mutating chunk_metas[other_chunk_index].
let import_refs: Vec<Ref> = chunk_metas[chunk_index].imports.keys().to_vec();
for import_ref in import_refs {
let imports = core::mem::take(&mut chunk_metas[chunk_index].imports);
for &import_ref in imports.keys() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is all run on one thread right? I don't remember

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, single-threaded. chunk_metas is a fn-local Vec<ChunkMeta> created at line 24 of compute_cross_chunk_dependencies, filled by the sequential for loop at line 78-80, then handed to this function as &mut [ChunkMeta]; it never leaves the call stack.

The walk step that esbuild runs per-chunk in goroutines is a plain sequential for chunk in chunks.iter_mut() here, so nothing else can observe chunk_metas[chunk_index].imports being empty between the mem::take and the restore at line 386. The body itself only touches chunk_metas[other_chunk_index].exports (guarded other_chunk_index != chunk_index at line 344) and chunks[chunk_index], never chunk_metas[*].imports.

let symbol = c.graph.symbols.get_const(import_ref).unwrap();

// Ignore uses that aren't top-level symbols
Expand Down Expand Up @@ -385,6 +383,7 @@ fn compute_cross_chunk_dependencies_with_chunk_metas(
}
}
}
chunk_metas[chunk_index].imports = imports;

// If this is an entry point, make sure we import all chunks belonging to
// this entry point, even if there are no imports. We need to make sure
Expand Down
2 changes: 1 addition & 1 deletion src/css/properties/flex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,11 +876,11 @@ impl FlexHandler {
}

if let (Some(g_val), Some(s_val), Some(b_val)) = (&mut grow, &mut shrink, &mut basis) {
// reshaped for borrowck
let g = g_val.0;
let g_prefix: &mut VendorPrefix = &mut g_val.1;
let s = s_val.0;
let s_prefix: &mut VendorPrefix = &mut s_val.1;
// basis is emitted again below as the FlexBasis longhand, so the shorthand needs its own copy.
let b = b_val.0.clone();
let b_prefix: &mut VendorPrefix = &mut b_val.1;

Expand Down
6 changes: 3 additions & 3 deletions src/dotenv/env_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,8 +1218,6 @@ impl<'a> Parser<'a> {
continue;
};
let value = self.parse_value::<IS_PROCESS>()?;
// reshaped for borrowck — value borrows self.value_buffer; copy before map mut.
let value_owned: Box<[u8]> = Box::from(value);
let entry = map.map.get_or_put(key)?;
if entry.found_existing {
if entry.index < count {
Expand All @@ -1231,7 +1229,9 @@ impl<'a> Parser<'a> {
}
// else: previous value freed by Drop on assignment below
}
*entry.value_ptr = HashTableValue { value: value_owned };
*entry.value_ptr = HashTableValue {
value: Box::from(value),
};
}
if !IS_PROCESS && EXPAND {
// borrowck — index-based iteration: clone the value bytes, run
Expand Down
Loading
Loading