Skip to content

Commit a37d3ab

Browse files
HKalbasicalebcartwright
authored andcommitted
Memoize format_expr
1 parent acdab00 commit a37d3ab

File tree

10 files changed

+11454
-4
lines changed

10 files changed

+11454
-4
lines changed

src/expr.rs

+50-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::borrow::Cow;
22
use std::cmp::min;
3+
use std::collections::HashMap;
34

45
use itertools::Itertools;
56
use rustc_ast::token::{DelimToken, LitKind};
@@ -22,7 +23,7 @@ use crate::macros::{rewrite_macro, MacroPosition};
2223
use crate::matches::rewrite_match;
2324
use crate::overflow::{self, IntoOverflowableItem, OverflowableItem};
2425
use crate::pairs::{rewrite_all_pairs, rewrite_pair, PairParts};
25-
use crate::rewrite::{Rewrite, RewriteContext};
26+
use crate::rewrite::{QueryId, Rewrite, RewriteContext};
2627
use crate::shape::{Indent, Shape};
2728
use crate::source_map::{LineRangeUtils, SpanUtils};
2829
use crate::spanned::Spanned;
@@ -53,6 +54,54 @@ pub(crate) fn format_expr(
5354
expr_type: ExprType,
5455
context: &RewriteContext<'_>,
5556
shape: Shape,
57+
) -> Option<String> {
58+
// when max_width is tight, we should check all possible formattings, in order to find
59+
// if we can fit expression in the limit. Doing it recursively takes exponential time
60+
// relative to input size, and people hit it with rustfmt takes minutes in #4476 #4867 #5128
61+
// By memoization of format_expr function, we format each pair of expression and shape
62+
// only once, so worst case execution time becomes O(n*max_width^3).
63+
if context.inside_macro() || context.is_macro_def {
64+
// span ids are not unique in macros, so we don't memoize result of them.
65+
return format_expr_inner(expr, expr_type, context, shape);
66+
}
67+
let clean;
68+
let query_id = QueryId {
69+
shape,
70+
span: expr.span,
71+
};
72+
if let Some(map) = context.memoize.take() {
73+
if let Some(r) = map.get(&query_id) {
74+
let r = r.clone();
75+
context.memoize.set(Some(map)); // restore map in the memoize cell for other users
76+
return r;
77+
}
78+
context.memoize.set(Some(map));
79+
clean = false;
80+
} else {
81+
context.memoize.set(Some(HashMap::default()));
82+
clean = true; // We got None, so we are the top level called function. When
83+
// this function finishes, no one is interested in what is in the map, because
84+
// all of them are sub expressions of this top level expression, and this is
85+
// done. So we should clean up memoize map to save some memory.
86+
}
87+
88+
let r = format_expr_inner(expr, expr_type, context, shape);
89+
if clean {
90+
context.memoize.set(None);
91+
} else {
92+
if let Some(mut map) = context.memoize.take() {
93+
map.insert(query_id, r.clone()); // insert the result in the memoize map
94+
context.memoize.set(Some(map)); // so it won't be computed again
95+
}
96+
}
97+
r
98+
}
99+
100+
fn format_expr_inner(
101+
expr: &ast::Expr,
102+
expr_type: ExprType,
103+
context: &RewriteContext<'_>,
104+
shape: Shape,
56105
) -> Option<String> {
57106
skip_out_of_file_lines_range!(context, expr.span);
58107

src/formatting.rs

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
use std::collections::HashMap;
44
use std::io::{self, Write};
5+
use std::rc::Rc;
56
use std::time::{Duration, Instant};
67

78
use rustc_ast::ast;
@@ -190,6 +191,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> {
190191
self.config,
191192
&snippet_provider,
192193
self.report.clone(),
194+
Rc::default(),
193195
);
194196
visitor.skip_context.update_with_attrs(&self.krate.attrs);
195197
visitor.is_macro_def = is_macro_def;

src/rewrite.rs

+13
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::shape::Shape;
1212
use crate::skip::SkipContext;
1313
use crate::visitor::SnippetProvider;
1414
use crate::FormatReport;
15+
use rustc_data_structures::stable_map::FxHashMap;
1516

1617
pub(crate) trait Rewrite {
1718
/// Rewrite self into shape.
@@ -24,10 +25,22 @@ impl<T: Rewrite> Rewrite for ptr::P<T> {
2425
}
2526
}
2627

28+
#[derive(Clone, PartialEq, Eq, Hash)]
29+
pub(crate) struct QueryId {
30+
pub(crate) shape: Shape,
31+
pub(crate) span: Span,
32+
}
33+
34+
// We use Option<HashMap> instead of HashMap, because in case of `None`
35+
// the function clean the memoize map, but it doesn't clean when
36+
// there is `Some(empty)`, so they are different.
37+
pub(crate) type Memoize = Rc<Cell<Option<FxHashMap<QueryId, Option<String>>>>>;
38+
2739
#[derive(Clone)]
2840
pub(crate) struct RewriteContext<'a> {
2941
pub(crate) parse_sess: &'a ParseSess,
3042
pub(crate) config: &'a Config,
43+
pub(crate) memoize: Memoize,
3144
pub(crate) inside_macro: Rc<Cell<bool>>,
3245
// Force block indent style even if we are using visual indent style.
3346
pub(crate) use_block: Cell<bool>,

src/shape.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::ops::{Add, Sub};
44

55
use crate::Config;
66

7-
#[derive(Copy, Clone, Debug)]
7+
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
88
pub(crate) struct Indent {
99
// Width of the block indent, in characters. Must be a multiple of
1010
// Config::tab_spaces.
@@ -139,7 +139,7 @@ impl Sub<usize> for Indent {
139139
// 8096 is close enough to infinite for rustfmt.
140140
const INFINITE_SHAPE_WIDTH: usize = 8096;
141141

142-
#[derive(Copy, Clone, Debug)]
142+
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
143143
pub(crate) struct Shape {
144144
pub(crate) width: usize,
145145
// The current indentation of code.

src/visitor.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::items::{
1717
use crate::macros::{macro_style, rewrite_macro, rewrite_macro_def, MacroPosition};
1818
use crate::modules::Module;
1919
use crate::parse::session::ParseSess;
20-
use crate::rewrite::{Rewrite, RewriteContext};
20+
use crate::rewrite::{Memoize, Rewrite, RewriteContext};
2121
use crate::shape::{Indent, Shape};
2222
use crate::skip::{is_skip_attr, SkipContext};
2323
use crate::source_map::{LineRangeUtils, SpanUtils};
@@ -71,6 +71,7 @@ impl SnippetProvider {
7171

7272
pub(crate) struct FmtVisitor<'a> {
7373
parent_context: Option<&'a RewriteContext<'a>>,
74+
pub(crate) memoize: Memoize,
7475
pub(crate) parse_sess: &'a ParseSess,
7576
pub(crate) buffer: String,
7677
pub(crate) last_pos: BytePos,
@@ -754,6 +755,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
754755
ctx.config,
755756
ctx.snippet_provider,
756757
ctx.report.clone(),
758+
ctx.memoize.clone(),
757759
);
758760
visitor.skip_context.update(ctx.skip_context.clone());
759761
visitor.set_parent_context(ctx);
@@ -765,10 +767,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
765767
config: &'a Config,
766768
snippet_provider: &'a SnippetProvider,
767769
report: FormatReport,
770+
memoize: Memoize,
768771
) -> FmtVisitor<'a> {
769772
FmtVisitor {
770773
parent_context: None,
771774
parse_sess: parse_session,
775+
memoize,
772776
buffer: String::with_capacity(snippet_provider.big_snippet.len() * 2),
773777
last_pos: BytePos(0),
774778
block_indent: Indent::empty(),
@@ -991,6 +995,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
991995
RewriteContext {
992996
parse_sess: self.parse_sess,
993997
config: self.config,
998+
memoize: self.memoize.clone(),
994999
inside_macro: Rc::new(Cell::new(false)),
9951000
use_block: Cell::new(false),
9961001
is_if_else_block: Cell::new(false),

0 commit comments

Comments
 (0)