Reduce cloning in language server

[overlookmotel]

While working on #13032, I noticed a lot of cloning going on Runtime::run_source (linter method used by language server).

oxc/crates/oxc_linter/src/service/runtime.rs

Lines 654 to 809 in 1585688

	#[cfg(feature = "language_server")]
	pub(super) fn run_source<'a>(
	&mut self,
	allocator: &'a mut oxc_allocator::Allocator,
	) -> Vec<MessageWithPosition<'a>> {
	use std::sync::Mutex;
	use oxc_data_structures::rope::Rope;
	use crate::{
	FixWithPosition,
	fixer::{Fix, PossibleFixesWithPosition},
	service::offset_to_position::{SpanPositionMessage, offset_to_position},
	};
	fn fix_to_fix_with_position<'a>(
	fix: &Fix<'a>,
	rope: &Rope,
	source_text: &str,
	) -> FixWithPosition<'a> {
	let start_position = offset_to_position(rope, fix.span.start, source_text);
	let end_position = offset_to_position(rope, fix.span.end, source_text);
	FixWithPosition {
	content: fix.content.clone(),
	span: SpanPositionMessage::new(start_position, end_position)
	.with_message(fix.message.as_ref().map(|label| Cow::Owned(label.to_string()))),
	}
	}
	// Wrap allocator in `MessageCloner` so can clone `Message`s into it
	let message_cloner = MessageCloner::new(allocator);
	let messages = Mutex::new(Vec::<MessageWithPosition<'a>>::new());
	let (sender, _receiver) = mpsc::channel();
	rayon::scope(|scope| {
	self.resolve_modules(scope, true, &sender, |me, mut module| {
	module.content.with_dependent_mut(
	|allocator_guard, ModuleContentDependent { source_text, section_contents }| {
	assert_eq!(module.section_module_records.len(), section_contents.len());
	let rope = &Rope::from_str(source_text);
	for (record_result, section) in module
	.section_module_records
	.into_iter()
	.zip(section_contents.drain(..))
	{
	let mut section_messages = match record_result {
	Err(diagnostics) => {
	messages
	.lock()
	.unwrap()
	.extend(diagnostics.into_iter().map(Into::into));
	continue;
	}
	Ok(module_record) => me.linter.run(
	Path::new(&module.path),
	Rc::new(section.semantic.unwrap()),
	Arc::clone(&module_record),
	allocator_guard,
	),
	};
	// adjust offset for multiple source text in a single file
	if section.source.start != 0 {
	for message in &mut section_messages {
	message
	.move_offset(section.source.start)
	.move_fix_offset(section.source.start);
	}
	}
	messages.lock().unwrap().extend(section_messages.iter().map(
	|message| {
	let message = message_cloner.clone_message(message);
	let labels = &message.error.labels.clone().map(|labels| {
	labels
	.into_iter()
	.map(|labeled_span| {
	let offset = labeled_span.offset() as u32;
	let start_position =
	offset_to_position(rope, offset, source_text);
	let end_position = offset_to_position(
	rope,
	offset + labeled_span.len() as u32,
	source_text,
	);
	let message = labeled_span
	.label()
	.map(|label| Cow::Owned(label.to_string()));
	SpanPositionMessage::new(
	start_position,
	end_position,
	)
	.with_message(message)
	})
	.collect::<Vec<_>>()
	});
	MessageWithPosition {
	message: message.error.message.clone(),
	severity: message.error.severity,
	help: message.error.help.clone(),
	url: message.error.url.clone(),
	code: message.error.code.clone(),
	labels: labels.clone(),
	fixes: match &message.fixes {
	PossibleFixes::None => PossibleFixesWithPosition::None,
	PossibleFixes::Single(fix) => {
	PossibleFixesWithPosition::Single(
	fix_to_fix_with_position(
	fix,
	rope,
	source_text,
	),
	)
	}
	PossibleFixes::Multiple(fixes) => {
	PossibleFixesWithPosition::Multiple(
	fixes
	.iter()
	.map(|fix| {
	fix_to_fix_with_position(
	fix,
	rope,
	source_text,
	)
	})
	.collect(),
	)
	}
	},
	}
	},
	));
	}
	},
	);
	});
	});
	// ToDo: oxc_diagnostic::Error is not compatible with MessageWithPosition
	// send use OxcDiagnostic or even better the MessageWithPosition struct
	// while let Ok(diagnostics) = receiver.recv() {
	// if let Some(diagnostics) = diagnostics {
	// messages.lock().unwrap().extend(
	// diagnostics.1
	// .into_iter()
	// .map(|report| MessageWithPosition::from(report))
	// );
	// }
	// }
	messages.into_inner().unwrap()
	}

It's not urgent, and neither I or @camc314 intend to address this now, but Cam thought it'd be useful to write up my observations so we (or someone else) can come back to this later.

Cloning

section_messages contains a bunch of Message<'a> objects. Each gets cloned (into provided Allocator), and then that cloned Message<'a> gets converted to a MessageWithPosition<'a>, which involves another round of cloning.

The ultimate caller of Runtime::run_source is in language server - IsolatedLintHandler::run_single. It takes the MessageWithPositions and converts them again to DiagnosticReport, which probably involves yet more cloning:

oxc/crates/oxc_language_server/src/linter/isolated_lint_handler.rs

Lines 97 to 101 in 1585688

	Some(self.lint_path(&mut allocator, &path, content).map_or(vec![], |errors| {
	let mut diagnostics: Vec<DiagnosticReport> = errors
	.iter()
	.map(|e| message_with_position_to_lsp_diagnostic_report(e, uri))
	.collect();

It might be a lot faster and simpler to skip all these intermediate conversions, and just produce DiagnosticReports in the first place.

Message<'a> lifetime

Message<'a> has a lifetime 'a because Fix<'a> does. Fix<'a> contains Cow<'a, str>s.

It's unclear to me in what circumstances a fix (or the suggestion message accompanying it) would reference 'a lifetime data (like a slice of the source text), without any modifications.

I wonder if these Cow<'a, str>s could be Cow<'static, str> instead, like in other structures in linter.

If so, that'd remove the need for Runtime::run_source to receive an Allocator, and cloning into that Allocator to extend the lifetimes would also be unnecessary.

Runtime::run_test_source

The same pattern of excessive cloning is also present in Runtime::run_test_source, but that's only used in tests, so less important to optimize it.

[overlookmotel]

Fixed in #14481.