Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deal with Idents/Names efficiently #3

Open
harpocrates opened this issue Jan 13, 2017 · 5 comments
Open

Deal with Idents/Names efficiently #3

harpocrates opened this issue Jan 13, 2017 · 5 comments

Comments

@harpocrates
Copy link
Owner

It may be the case that identifiers or names should be interned, or use Text, but in any case the current situation should change.

@harpocrates
Copy link
Owner Author

harpocrates commented Jan 15, 2017

Might be worth considering an Invalid constructor given the number of times Rust relies on that idea. Either that, or adjust the AST to get rid of this pattern (and have mkIdent prevent this case at runtime).

data Ident = Invalid | Ident String -- or some ID in a global symbol table

For example, an Arg requires a pattern, but for cases like fn f(i32) -> i32, that pattern is an IdentP of an invalid (empty) Ident. Similarly, in FieldPat, there is an edge case where there is no identifier. Rust fills it in with the same identifier as the patter.

@harpocrates
Copy link
Owner Author

For now, Ident stayed the same, and instead the AST was adjusted to avoid having to fill Ident holes with an empty/filler/invalid Ident. Still not ruling out the possibility of adding the Invalid :: Ident variant, especially since it would match more closely the Rust AST.

@harpocrates
Copy link
Owner Author

Reopening this for the more general problem of how to efficiently deal with Ident and Name.

@harpocrates
Copy link
Owner Author

harpocrates commented Mar 15, 2017

This commit removed the Name data type and just changed it to be a synonym for String. After a couple more simplifications, things ended up pretty nice. This issue is now repurposed for the question of whether it is worth trying to compact strings?

For example, have something like

import Data.IntMap

mkIdent :: IntMap Ident -> String -> (IntMap Ident, Ident)
mkIdent s cache = case lookup h cache of
                           Just ident@(Ident s' _) | s == s' -> (cache, ident)
                           _ -> let ident = Ident s h in  (insert h ident cache, ident)
     where h = hash s

The cost of the IntMap probably dwarfs any benefits of using it though... Still something to keep in mind.

@harpocrates harpocrates changed the title Deal with Idents/Names properly Deal with Idents/Names efficiently Oct 13, 2017
@harpocrates
Copy link
Owner Author

There are really two ideas to keep track of here:

  1. Compacting common Name's (like the previous comment)
  2. Representing Name's in a format that saves bytes and pointer indirection (like Text)

I should benchmark this before doing anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant