apache / apache/datafusion-sqlparser-rs
Improve performance by not copying `Token`s as much
- Ngôn ngữ chính
- Rust
- Star
- 3.5k
- Fork
- 772
- Merge trung bình
- 4 ngày 9 giờ
- Pull request đã merge (30 ngày)
- 17
Mô tả
Part of https://github.com/apache/datafusion-sqlparser-rs/issues/1557
While looking at the flamegraph posted to that ticket, one obvious source of improvement is to stop copying each token so much.
Functions like `peek_token()`, and `expect_token()` copy the `Token` (which often includes a string)
```rust
impl Parser {
// returns an OWNED TokenWithLocation
pub fn peek_token(&self) -> TokenWithLocation {
...
}
}
```
For example
https://github.com/apache/datafusion-sqlparser-rs/blob/2e90e105a74bf9f50f2bad6c22992759ddb06880/src/parser/mod.rs#L3314-L3334
In the above code, the `non_whitespace.cloned()` call actually copies the token (and string) even when many callsites only need to check what it is and could get by with a &
## Suggestions for improvements
The biggest suggestion is to stop copying each token unless it actually needed a clone. For example instead of this
```rust
impl Parser {
// returns an OWNED TokenWithLocation
pub fn peek_token(&self) -> TokenWithLocation {
...
}
}
```
Make it like this
```rust
impl Parser {
// returns an reference to TokenWithLocation
// (the & means no copying!)
pub fn peek_token_ref(&self) -> &TokenWithLocation {
...
}
}
```
I think we could do this without massive breaking changes by adding new functions like `peek_token_ref()` that returned a reference to the token rather than a clone
## Ideas
I played around with it a bit and here is what I came up with. I think a similar pattern could be applied to other places
```rust
impl Parser {
...
/// Return the first non-whitespace token that has not yet been processed
/// (or None if reached end-of-file) and mark it as processed. OK to call
/// repeatedly after reaching EOF.
pub fn next_token(&mut self) -> TokenWithLocation {
self.next_token_ref().clone()
}
/// Return the first non-whitespace token that has not yet been processed
/// (or None if reached end-of-file) and mark it as processed. OK to call
/// repeatedly after reaching EOF.
pub fn next_token_ref(&mut self) -> &TokenWithLocation {
loop {
self.index += 1;
// skip whitespace
if let Some(TokenWithLocation {
token: Token::Whitespace(_),
location: _,
}) = self.tokens.get(self.index - 1) {
continue
}
break;
}
if (self.index - 1) < self.tokens.len() {
&self.tokens[self.index - 1]
}
else {
eof_token()
}
}
...
}
static EOF_TOKEN: OnceLock = OnceLock::new();
fn eof_token() -> &'static TokenWithLocation {
EOF_TOKEN.get_or_init(|| {
TokenWithLocation::wrap(Token::EOF)
})
}
```
Hướng dẫn đóng góp
Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này
Hướng nghiên cứu
Bắt đầu trong src/parser/mod.rs, đặc biệt là peek_token(), expect_token() và lời gọi non_whitespace.cloned() được xác định trong issue. Theo dõi những vị trí gọi chỉ kiểm tra các token so với những vị trí yêu cầu các giá trị được sở hữu, đồng thời giữ nguyên hành vi hiện có của parser trong khi loại bỏ các bản sao Token không cần thiết; công việc hoàn tất khi các đường dẫn đó tránh được việc clone dư thừa mà không làm hỏng các API hiện có.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- rust
- Lĩnh vực
- compilers, performance
- Loại issue
- Tái cấu trúc
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức độ hoạt động
- Đình trệ
- Độ rõ ràng
- Khá rõ ràng
- Mức phù hợp với người mới
- 35/100