Skip to content

Commit

Permalink
perf: Use FxHash (#1224)
Browse files Browse the repository at this point in the history
This PR introduces the `FxHashMap`, `FxHashSet`, `FxIndexMap` and `FxIndexSet` types which use the non-cryptographic [fxhash](https://github.com/rust-lang/rustc-hash) algorithm also used in the Rust compiler. Doing so improves the `plc check` times by ~300 to 500 ms in a bigger internal project of ours.
  • Loading branch information
volsa authored May 22, 2024
1 parent 49f8749 commit 406457a
Show file tree
Hide file tree
Showing 38 changed files with 237 additions and 214 deletions.
13 changes: 12 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,14 @@ lazy_static.workspace = true
serde_json.workspace = true
serde.workspace = true
toml.workspace = true
rustc-hash.workspace = true

[dev-dependencies]
num = "0.4"
insta = "1.31.0"
pretty_assertions = "1.3.0"
driver = { path = "./compiler/plc_driver/", package = "plc_driver" }
project = { path = "./compiler/plc_project/", package = "plc_project", features = ["integration"]}
project = { path = "./compiler/plc_project/", package = "plc_project", features = ["integration"] }
plc_xml = { path = "./compiler/plc_xml" }
serial_test = "*"
tempfile = "3"
Expand Down Expand Up @@ -90,3 +91,4 @@ lazy_static = "1.4.0"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1"
toml = "0.5"
rustc-hash = "1.1.0"
3 changes: 2 additions & 1 deletion compiler/plc_ast/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition = "2021"

[dependencies]
plc_util = { path = "../plc_util" }
plc_source = {path = "../plc_source"}
plc_source = { path = "../plc_source" }
chrono = { version = "0.4", default-features = false }
serde = { version = "1.0", features = ["derive"] }
rustc-hash.workspace = true
6 changes: 3 additions & 3 deletions compiler/plc_ast/src/pre_processor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) 2020 Ghaith Hachem and Mathias Rieder

use std::collections::HashMap;
use rustc_hash::FxHashMap;

use plc_util::convention::internal_type_name;

Expand Down Expand Up @@ -176,7 +176,7 @@ fn build_enum_initializer(
}

fn preprocess_generic_structs(pou: &mut Pou) -> Vec<UserTypeDeclaration> {
let mut generic_types = HashMap::new();
let mut generic_types = FxHashMap::default();
let mut types = vec![];
for binding in &pou.generics {
let new_name = format!("__{}__{}", pou.name, binding.name); // TODO: Naming convention (see plc_util/src/convention.rs)
Expand Down Expand Up @@ -269,7 +269,7 @@ fn add_nested_datatypes(
}
}

fn replace_generic_type_name(dt: &mut DataTypeDeclaration, generics: &HashMap<String, String>) {
fn replace_generic_type_name(dt: &mut DataTypeDeclaration, generics: &FxHashMap<String, String>) {
match dt {
DataTypeDeclaration::DataTypeDefinition { data_type, .. } => match data_type {
DataType::ArrayType { referenced_type, .. }
Expand Down
1 change: 1 addition & 0 deletions compiler/plc_diagnostics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ toml.workspace = true
anyhow.workspace = true
lazy_static.workspace = true
log.workspace = true
rustc-hash.workspace = true
12 changes: 6 additions & 6 deletions compiler/plc_diagnostics/src/diagnostician.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use rustc_hash::FxHashMap;

use crate::{
diagnostics::{
Expand All @@ -16,7 +16,7 @@ use crate::{
pub struct Diagnostician {
reporter: Box<dyn DiagnosticReporter>,
assessor: Box<dyn DiagnosticAssessor>,
filename_fileid_mapping: HashMap<String, usize>,
filename_fileid_mapping: FxHashMap<String, usize>,
}

impl Diagnostician {
Expand Down Expand Up @@ -75,7 +75,7 @@ impl Diagnostician {
Diagnostician {
assessor: Box::<DiagnosticsRegistry>::default(),
reporter: Box::<NullDiagnosticReporter>::default(),
filename_fileid_mapping: HashMap::new(),
filename_fileid_mapping: FxHashMap::default(),
}
}

Expand All @@ -84,7 +84,7 @@ impl Diagnostician {
Diagnostician {
assessor: Box::<DiagnosticsRegistry>::default(),
reporter: Box::new(CodeSpanDiagnosticReporter::buffered()),
filename_fileid_mapping: HashMap::new(),
filename_fileid_mapping: FxHashMap::default(),
}
}

Expand All @@ -93,7 +93,7 @@ impl Diagnostician {
Diagnostician {
reporter: Box::<ClangFormatDiagnosticReporter>::default(),
assessor: Box::<DiagnosticsRegistry>::default(),
filename_fileid_mapping: HashMap::new(),
filename_fileid_mapping: FxHashMap::default(),
}
}

Expand Down Expand Up @@ -151,7 +151,7 @@ impl Default for Diagnostician {
Self {
reporter: Box::<CodeSpanDiagnosticReporter>::default(),
assessor: Box::<DiagnosticsRegistry>::default(),
filename_fileid_mapping: HashMap::new(),
filename_fileid_mapping: FxHashMap::default(),
}
}
}
Expand Down
13 changes: 6 additions & 7 deletions compiler/plc_diagnostics/src/diagnostics/diagnostics_registry.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/// Returns a diagnostics map with the error code, default severity and a description
use std::collections::HashMap;

use lazy_static::lazy_static;
use rustc_hash::FxHashMap;
use serde::{Deserialize, Serialize};

use crate::diagnostician::DiagnosticAssessor;
Expand All @@ -13,7 +12,7 @@ use super::{

macro_rules! add_diagnostic {
($($number:ident, $severity:expr, $desc:expr,)*) => {
{ let mut m : HashMap<&str, DiagnosticEntry> = HashMap::new();
{ let mut m : FxHashMap<&str, DiagnosticEntry> = FxHashMap::default();
$(
{
let code = stringify!($number);
Expand All @@ -24,7 +23,7 @@ macro_rules! add_diagnostic {
}}
}

pub struct DiagnosticsRegistry(HashMap<&'static str, DiagnosticEntry>);
pub struct DiagnosticsRegistry(FxHashMap<&'static str, DiagnosticEntry>);

#[derive(Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct DiagnosticEntry {
Expand All @@ -41,7 +40,7 @@ impl Default for DiagnosticsRegistry {

impl DiagnosticsRegistry {
/// Creates an empty registry.
fn new(map: HashMap<&'static str, DiagnosticEntry>) -> Self {
fn new(map: FxHashMap<&'static str, DiagnosticEntry>) -> Self {
DiagnosticsRegistry(map)
}

Expand Down Expand Up @@ -87,7 +86,7 @@ impl DiagnosticAssessor for DiagnosticsRegistry {

#[derive(Serialize, Deserialize, PartialEq, Eq, Default)]
#[serde(transparent)]
pub struct DiagnosticsConfiguration(HashMap<Severity, Vec<String>>);
pub struct DiagnosticsConfiguration(FxHashMap<Severity, Vec<String>>);

impl From<&DiagnosticsRegistry> for DiagnosticsConfiguration {
fn from(registry: &DiagnosticsRegistry) -> Self {
Expand All @@ -101,7 +100,7 @@ impl From<&DiagnosticsRegistry> for DiagnosticsConfiguration {
}

lazy_static! {
static ref DIAGNOSTICS: HashMap<&'static str, DiagnosticEntry> = add_diagnostic!(
static ref DIAGNOSTICS: FxHashMap<&'static str, DiagnosticEntry> = add_diagnostic!(
E001,
Error,
include_str!("./error_codes/E001.md"), //General Error
Expand Down
7 changes: 4 additions & 3 deletions compiler/plc_driver/src/pipelines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use ast::{
ast::{pre_process, CompilationUnit, LinkageType},
provider::IdProvider,
};
use indexmap::IndexSet;

use plc::index::FxIndexSet;
use plc::{
codegen::{CodegenContext, GeneratedModule},
index::Index,
Expand Down Expand Up @@ -192,7 +193,7 @@ impl<T: SourceContainer + Sync> IndexedProject<T> {
/// A project that has been annotated with information about different types and used units
pub struct AnnotatedProject<T: SourceContainer + Sync> {
pub project: Project<T>,
pub units: Vec<(CompilationUnit, IndexSet<Dependency>, StringLiterals)>,
pub units: Vec<(CompilationUnit, FxIndexSet<Dependency>, StringLiterals)>,
pub index: Index,
pub annotations: AstAnnotations,
}
Expand Down Expand Up @@ -266,7 +267,7 @@ impl<T: SourceContainer + Sync> AnnotatedProject<T> {
context: &'ctx CodegenContext,
compile_options: &CompileOptions,
unit: &CompilationUnit,
dependencies: &IndexSet<Dependency>,
dependencies: &FxIndexSet<Dependency>,
literals: &StringLiterals,
) -> Result<GeneratedModule<'ctx>, Diagnostic> {
let mut code_generator = plc::codegen::CodeGen::new(
Expand Down
1 change: 1 addition & 0 deletions compiler/plc_index/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ plc_ast = { path = "../plc_ast" }
plc_source = { path = "../plc_source" }
plc_diagnostics = { path = "../plc_diagnostics" }
#plc_project = { path = "../plc_project" } # TODO: This will create a circular dependency
rustc-hash.workspace = true

encoding_rs = "0.8"
6 changes: 3 additions & 3 deletions compiler/plc_index/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ use plc_ast::provider::IdProvider;
use plc_diagnostics::diagnostics::Diagnostic;
use plc_source::source_location::SourceLocation;
use plc_source::{SourceCode, SourceContainer};
use std::collections::HashMap;
use rustc_hash::FxHashMap;

#[derive(Debug, Default)]
pub struct GlobalContext {
/// HashMap containing all read, i.e. parsed, sources where the key represents
/// the relative file path and the value some [`SourceCode`]
sources: HashMap<&'static str, SourceCode>,
sources: FxHashMap<&'static str, SourceCode>,

/// [`IdProvider`] used during the parsing session
provider: IdProvider,
Expand All @@ -24,7 +24,7 @@ pub struct GlobalContext {

impl GlobalContext {
pub fn new() -> Self {
Self { sources: HashMap::new(), provider: IdProvider::default() }
Self { sources: FxHashMap::default(), provider: IdProvider::default() }
}

/// Inserts a single [`SourceCode`] to the internal source map
Expand Down
1 change: 1 addition & 0 deletions compiler/plc_xml/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ast = { path = "../plc_ast/", package = "plc_ast" }
plc_diagnostics = { path = "../plc_diagnostics" }
plc_source = { path = "../plc_source" }
itertools.workspace = true
rustc-hash.workspace = true

[features]
default = []
Expand Down
5 changes: 3 additions & 2 deletions compiler/plc_xml/src/extensions.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! In this file extension traits are defined to be used within the `plc_xml` crate.
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;

use quick_xml::name::QName;
use rustc_hash::FxHashMap;

use crate::error::Error;

Expand Down Expand Up @@ -34,7 +35,7 @@ impl TryToString for Cow<'_, [u8]> {
}
}

impl GetOrErr for HashMap<String, String> {
impl GetOrErr for FxHashMap<String, String> {
fn get_or_err(&self, key: &str) -> Result<String, Error> {
self.get(key).map(|it| it.to_owned()).ok_or(Error::MissingAttribute(key.to_string()))
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/plc_xml/src/model/block.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;

use quick_xml::events::{BytesStart, Event};
use rustc_hash::FxHashMap;

use crate::{
error::Error,
Expand All @@ -21,7 +22,7 @@ pub(crate) struct Block<'xml> {
}

impl<'xml> Block<'xml> {
pub fn new(mut hm: HashMap<String, String>, variables: Vec<BlockVariable>) -> Result<Self, Error> {
pub fn new(mut hm: FxHashMap<String, String>, variables: Vec<BlockVariable>) -> Result<Self, Error> {
Ok(Self {
local_id: hm.get_or_err("localId").map(|it| it.parse())??,
type_name: Cow::from(hm.get_or_err("typeName")?),
Expand Down
5 changes: 3 additions & 2 deletions compiler/plc_xml/src/model/connector.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{borrow::Cow, collections::HashMap};
use std::borrow::Cow;

use quick_xml::events::Event;
use rustc_hash::FxHashMap;

use crate::{
error::Error,
Expand All @@ -19,7 +20,7 @@ pub(crate) struct Connector<'xml> {
}

impl<'xml> Connector<'xml> {
pub fn new(mut hm: HashMap<String, String>, kind: ConnectorKind) -> Result<Self, Error> {
pub fn new(mut hm: FxHashMap<String, String>, kind: ConnectorKind) -> Result<Self, Error> {
Ok(Self {
kind,
name: Cow::from(hm.get_or_err("name")?),
Expand Down
5 changes: 3 additions & 2 deletions compiler/plc_xml/src/model/control.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{borrow::Cow, collections::HashMap, str::FromStr};
use std::{borrow::Cow, str::FromStr};

use quick_xml::events::Event;
use rustc_hash::FxHashMap;

use crate::{
error::Error,
Expand All @@ -20,7 +21,7 @@ pub(crate) struct Control<'xml> {
}

impl<'xml> Control<'xml> {
pub fn new(mut hm: HashMap<String, String>, kind: ControlKind) -> Result<Self, Error> {
pub fn new(mut hm: FxHashMap<String, String>, kind: ControlKind) -> Result<Self, Error> {
Ok(Self {
kind,
name: hm.remove("label").map(Cow::from),
Expand Down
Loading

0 comments on commit 406457a

Please sign in to comment.