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.

Sign up for GitHub

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

Jump to bottom

Feature/change hash lib #39

Merged
merged 6 commits into from
Aug 12, 2021
Merged

Feature/change hash lib #39

merged 6 commits into from
Aug 12, 2021

Conversation

Gorlph
Copy link
Contributor

@Gorlph Gorlph commented Aug 11, 2021 •
edited by bact
Loading

This PR has 2 major changes

  1. Change hashmap and hashset from ahash to rustc_hash, which improves speed a little bit.
  2. (Incrementally) improve error handling in Newmm functions - internal_segment and other functions now return Result. Tokenizer trait is also changed.

Cargo [profile.release] configuration is experimental, I try a method from The Performance Book

Need help with benchmarking.

Will also close #38

@Gorlph Gorlph mentioned this pull request Aug 11, 2021
Open
@bact bact added the enhancement New feature or request label Aug 11, 2021
@bact
Copy link
Member

bact commented Aug 11, 2021

Have several warnings here:

src/tokenizer/tcc_custom.rs#L104
function is never used: `tcc_pos`

src/tokenizer/tcc_custom.rs#L121
function is never used: `segment`

src/tokenizer/dict_reader_custom.rs#L11
variant is never constructed: `WordList`

for tcc_pos, probably removing the old tcc_pos and renaming tcc_pos_quick to (new) tcc_pos?

bact and others added 2 commits August 12, 2021 10:22
@bact
Sort dependencies
31450ba
@Gorlph
rename tcc_pos_quick to tcc_pos
26afbab 
tcc segment is renamed to tcc_segment - allowed as dead code
@bact bact merged commit 761a399 into main Aug 12, 2021
@bact bact mentioned this pull request Aug 12, 2021
Merged
@bact bact deleted the feature/change-hash-lib branch November 8, 2021 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clarify method in Tokenizer trait
2 participants
@Gorlph @bact