Skip to content

Commit

Permalink
hparser: convert locations in increasing order
Browse files Browse the repository at this point in the history
Summary:
Converting locations in strictly increasing order dramatically improves
location cache utilization and performance. Previously we were
converting the parent start and end location before recursively
converting the children. That means were were jumping from the start to
end and then back to the middle instead of going in increasing order.

Now we convert the parent start location first, convert the children
recursively, and finally in the end convert the parent end location.

In tests caching alone improved performance by 82%. The present change
further improved performance by another 83%! The total improvement from
both optimization was 234% (the new code is 3.34 times faster).

Reviewed By: avp

Differential Revision: D31044165

fbshipit-source-id: 06fa1aa2489fecc205d53e1f01d365dd1b8d40bf
  • Loading branch information
tmikov authored and facebook-github-bot committed Sep 23, 2021
1 parent b4a7b1a commit 760a22b
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 17 deletions.
12 changes: 2 additions & 10 deletions unsupported/juno/src/hparser/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use super::generated_cvt::cvt_node_ptr;
use crate::ast;
use hermes::parser::{
DataRef, HermesParser, NodeLabel, NodeLabelOpt, NodeListOptRef, NodeListRef, NodePtr,
NodePtrOpt, NodeString, NodeStringOpt, SMLoc, SMRange,
NodePtrOpt, NodeString, NodeStringOpt, SMLoc,
};
use hermes::utf::{
is_utf8_continuation, utf8_with_surrogates_to_string, utf8_with_surrogates_to_utf16,
Expand Down Expand Up @@ -88,19 +88,11 @@ impl Converter<'_> {
}
}

pub fn smrange(&mut self, smr: SMRange) -> ast::SourceRange {
ast::SourceRange {
file: self.file_id,
start: self.cvt_smloc(smr.start),
end: self.cvt_smloc(smr.end.pred()),
}
}

/// Convert a SMLoc to ast::SourceLoc using the line_cache. Best cache
/// utilization is achieved if locations are always increasing. In this way
/// the next location will almost always be in the current line or the next
/// line.
fn cvt_smloc(&mut self, loc: SMLoc) -> ast::SourceLoc {
pub fn cvt_smloc(&mut self, loc: SMLoc) -> ast::SourceLoc {
assert!(
loc.is_valid(),
"All source locations from Hermes parser must be valid"
Expand Down
15 changes: 11 additions & 4 deletions unsupported/juno/src/hparser/generated_cvt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ use crate::ast;

pub unsafe fn cvt_node_ptr(cvt: &mut Converter, n: NodePtr) -> ast::NodePtr {
let nr = n.as_ref();
let range = cvt.smrange(nr.source_range);
let range = ast::SourceRange {
file: cvt.file_id,
start: cvt.cvt_smloc(nr.source_range.start),
end: ast::SourceLoc::invalid(),
};

match nr.kind {
let mut res = match nr.kind {
NodeKind::Empty => ast::NodePtr::new(
ast::Node {
range,
Expand Down Expand Up @@ -1862,5 +1866,8 @@ pub unsafe fn cvt_node_ptr(cvt: &mut Converter, n: NodePtr) -> ast::NodePtr {
}
),
_ => panic!("Invalid node kind")
}
}
};

res.range.end = cvt.cvt_smloc(nr.source_range.end.pred());

res}
14 changes: 11 additions & 3 deletions unsupported/tools/rustgen/rustgen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,14 @@ static void genConvert() {
llvh::outs()
<< "pub unsafe fn cvt_node_ptr(cvt: &mut Converter, n: NodePtr) -> ast::NodePtr {\n";
llvh::outs() << " let nr = n.as_ref();\n"
" let range = cvt.smrange(nr.source_range);\n"
" let range = ast::SourceRange {\n"
" file: cvt.file_id,\n"
" start: cvt.cvt_smloc(nr.source_range.start),\n"
" end: ast::SourceLoc::invalid(),\n"
" };\n"
"\n";

llvh::outs() << " match nr.kind {\n";
llvh::outs() << " let mut res = match nr.kind {\n";

auto genStruct = [](const TreeClass &cls) {
if (cls.sentinel != SentinelType::None)
Expand Down Expand Up @@ -451,7 +455,11 @@ static void genConvert() {
}

llvh::outs() << " _ => panic!(\"Invalid node kind\")\n"
" }\n";
" };\n\n";
llvh::outs()
<< " res.range.end = cvt.cvt_smloc(nr.source_range.end.pred());\n"
"\n"
" res";
llvh::outs() << "}\n";
}

Expand Down

0 comments on commit 760a22b

Please sign in to comment.