Interned all strings.

[orizi]

This change is 

[mkaput]

Reviewable status: 0 of 124 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware, @ilyalesokhin-starkware, and @orizi)

a discussion (no related file):
This change sounds like it will create a massive memory leak-style behaviour, especially in the LS. Salsa has no means for garbage collection for interned items (LRU doesn't work there), so it will hard for me to combat in LS.

---

[ilyalesokhin-starkware]

Why did you remove all the Smolstr usage?

[ilyalesokhin-starkware]

crates/cairo-lang-syntax/src/node/ids.rs line 12 at r1 (raw file):

use crate::node::stable_ptr::SyntaxStablePtr;

define_short_id!(TextId, Arc::<str>, SyntaxGroup, lookup_intern_text, intern_text);

Maybe StringId?
(non-blocking)

Suggestion:

StringId

[ilyalesokhin-starkware]

crates/cairo-lang-semantic/src/items/modifiers.rs line 16 at r1 (raw file):

    modifier_list: &[Modifier],
) -> Mutability {
    let mut last: Option<&Modifier> = None;

why is this change is the same PR?

Code quote:

let mut last: Option<&Modifier> = None;

[ilyalesokhin-starkware]

crates/cairo-lang-semantic/src/resolve/mod.rs line 502 at r1 (raw file):

            ast::PathSegment::WithGenericArgs(_) => false,
            ast::PathSegment::Simple(simple) => {
                simple.ident(syntax_db).text(syntax_db) == TextId::interned("super", self.db)

Suggestion:

simple.ident(syntax_db).text(syntax_db).lookup_intern(self.db).as_ref() == "super"

[ilyalesokhin-starkware]

Reviewed 92 of 124 files at r1, all commit messages.
Reviewable status: 92 of 124 files reviewed, 4 unresolved discussions (waiting on @gilbens-starkware and @orizi)

---

crates/cairo-lang-semantic/src/resolve/mod.rs line 502 at r1 (raw file):

            ast::PathSegment::WithGenericArgs(_) => false,
            ast::PathSegment::Simple(simple) => {
                simple.ident(syntax_db).text(syntax_db) == TextId::interned("super", self.db)

other places as well.

[orizi]

Most of it should just have become the inner arc - there may be some spill over.
in any case - it seems the over abundance of arc is enough of an issue to make this not really worth it.

Reviewable status: 92 of 124 files reviewed, 4 unresolved discussions (waiting on @gilbens-starkware, @ilyalesokhin-starkware, and @mkaput)

a discussion (no related file):

Previously, mkaput (Marek Kaput) wrote…

This change sounds like it will create a massive memory leak-style behaviour, especially in the LS. Salsa has no means for garbage collection for interned items (LRU doesn't work there), so it will hard for me to combat in LS.

this is no worse than the entire existing db - only removing the duplication of data.

---

---

crates/cairo-lang-semantic/src/items/modifiers.rs line 16 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

why is this change is the same PR?

it was removal of being text dependent.

[mkaput]

Reviewable status: 92 of 124 files reviewed, 4 unresolved discussions (waiting on @gilbens-starkware, @ilyalesokhin-starkware, and @orizi)

a discussion (no related file):

Previously, orizi wrote…

this is no worse than the entire existing db - only removing the duplication of data.

Yes, it is no worse, but it's also not better, and I think it'd be better to only take steps towards better memory management?

Have you measured how much memory is actually deduplicated here? Note that Arc<str> and SmolStr do also deduplicate clones, yet they properly deallocate when the last instance is dropped.

---

[mkaput]

Reviewed 124 of 124 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @gilbens-starkware, @ilyalesokhin-starkware, and @orizi)

a discussion (no related file):
FYI https://doc.rust-lang.org/stable/alloc/sync/struct.Arc.html#impl-Default-for-Arc%3Cstr%3E stabilized in Rust 1.80

---