perf(core): Cache source lookups

[nayeemrmn]

Keep a cache for source maps and source lines. We sort of already had a cache argument for source map lookup functions but we just passed an empty map instead of storing it (even before moving to core). I extended it to cache source line lookups as well and plugged it into runtime state.

cc @bartlomieju This runtime state cache extends beyond caching across frame lookups for a single stack, and we don't need to accept a list of locations in op_apply_source_map. (from discord)

[bartlomieju]

cc @bartlomieju This runtime state cache extends beyond caching across frame lookups for a single stack, and we don't need to accept a list of locations in op_apply_source_map. (from discord)

I was thinking about that, when reading through source code. On one hand that seems like a no-brainer, but on the other these sources will be cached indefinitely in memory (ie. until the program exits). I'm not sure what will be the memory impact, but we should be careful about that. Additionally it appears we are doing a lot of copying when acquiring the source map:

deno/cli/proc_state.rs

Lines 697 to 703 in 94068b7

	if let Some((code, maybe_map)) = self.get_emit(&specifier) {
	let code = String::from_utf8(code).unwrap();
	source_map_from_code(code).or(maybe_map)
	} else if let Ok(source) = self.load(specifier, None, false) {
	let code = String::from_utf8(source.code.to_vec()).unwrap();
	source_map_from_code(code)
	} else {

@dsherret @kitsonk @lucacasonato do you have thoughts on the topic?

[nayeemrmn]

I'm not sure what will be the memory impact, but we should be careful about that.

We already cache all sources in memory, this makes it also cache the source map for the subset of those which are referenced in any stack trace. I think programs have a finite and small amount of unique rendered stack traces. With the source line cache I made sure to move the 150 character cap to core so we don't cache anything ridiculous like from minified code.

[bartlomieju]

I'm not sure what will be the memory impact, but we should be careful about that.

We already cache all sources in memory, this makes it also cache the source map for the subset of those which are referenced in any stack trace. I think programs have a finite and small amount of unique rendered stack traces. With the source line cache I made sure to move the 150 character cap to core so we don't cache anything ridiculous like from minified code.

Do we actually cache source maps in memory? Could you point me to relevant code?

[nayeemrmn]

Do we actually cache source maps in memory?

We cache sources already, I'm saying adding the source map for a small (and ~constant wrt program size IMO) subset of those wouldn't be a big deal

[bartlomieju]

@nayeemrmn this PR looks fine to me, there's one more quirk that would be great to be addressed.

In we're converting source code to a string, then iterating over the lines to find a source map and create another string out of it to be decoded. I believe all of this could be done on a vector of bytes instead to avoid many round-trip in string <-> bytes conversion. Could you look into that?

deno/cli/proc_state.rs

Lines 759 to 777 in 60869c2

	fn source_map_from_code(code: String) -> Option<Vec<u8>> {
	let lines: Vec<&str> = code.split('\n').collect();
	if let Some(last_line) = lines.last() {
	if last_line
	.starts_with("//# sourceMappingURL=data:application/json;base64,")
	{
	let input = last_line.trim_start_matches(
	"//# sourceMappingURL=data:application/json;base64,",
	);
	let decoded_map = base64::decode(input)
	.expect("Unable to decode source map from emitted file.");
	Some(decoded_map)
	} else {
	None
	}
	} else {
	None
	}
	}

[bartlomieju]

LGTM, thank you @nayeemrmn, great clean up