Add litemap crate (TupleVecMap) #496

Manishearth · 2021-02-17T01:50:48Z

Fixes #484

Somewhat WIP: I want to add doctests and use it in icu4x itself.

This is currently in tree, eventually I think this should be maintained out-of-tree as a general purpose utility, but for now I didn't want to figure that out and it should be fine here.

@zbraniecki: I'm not super happy with the name, I picked it as a kind of ironic "haha, it's a flat map" thing, but I couldn't think of great names. I would have liked mercator since it's a map projection, but that's taken, and the obvious vec-map is too. Unsure if the type itself should be called FlattenedMap or something, and FlatMap has another connotation.

utils/terrain/src/map.rs

echeran

LGTM.

For bonus points in the future, you can see if there's a threshold of the size under which it's faster to just iterate over the elements instead of binary search. For Clojure's array-map, it's apparently 9, and then it auto-upgrades itself to a BTreeMap-esque Map (HAMT).

utils/terrain/src/map.rs

jira-pull-request-webhook · 2021-02-17T16:48:00Z

Notice: the branch changed across the force-push!

utils/terrain/src/map.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

jira-pull-request-webhook · 2021-02-17T16:59:54Z

Notice: the branch changed across the force-push!

utils/terrain/src/map.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

coveralls · 2021-02-17T17:01:15Z

Pull Request Test Coverage Report for Build 040c69ba2da56a7a79fec9f291bc5081b7b4e63b-PR-496

1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.004%) to 72.752%

Totals
Change from base Build 3be597cdf89432e493e7777bb4dec54854a47f97:	0.004%
Covered Lines:	4886
Relevant Lines:	6716

💛 - Coveralls

codecov-io · 2021-02-17T17:01:44Z

Codecov Report

Merging #496 (e3c1f6d) into master (3be597c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #496   +/-   ##
=======================================
  Coverage   73.35%   73.36%           
=======================================
  Files         117      118    +1     
  Lines        6501     6502    +1     
=======================================
+ Hits         4769     4770    +1     
  Misses       1732     1732

Impacted Files	Coverage Δ
utils/terrain/src/lib.rs	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3be597c...8f2a5ac. Read the comment docs.

Manishearth · 2021-02-17T23:38:52Z

Added doctests. Are there specific places folks want me to use this? Also, I haven't added a requirement that all keys be Copy, but I could.

Another potential performance win is splitting the vectors into two for cache reasons, but I don't think it will matter unless V is very big.

utils/terrain/Cargo.toml

utils/terrain/src/lib.rs

utils/terrain/src/map.rs

zbraniecki

lgtm!

sffc · 2021-02-18T00:19:33Z

utils/terrain/Cargo.toml

+# (online at: https://github.com/unicode-org/icu4x/blob/master/LICENSE ).
+
+[package]
+name = "terrain"


I have a small to moderate preference for using a functional name like "sorted_vec_map", only because "terrain" isn't really that good of a name for what we're building.

utils/terrain/src/map.rs

sffc

Two things:

You need to run cargo fmt
Let's decide the name of the crate in the meeting tomorrow

Manishearth · 2021-02-18T20:41:49Z

Why does CI fail on Cargo.lock changes?

sffc · 2021-02-19T02:51:51Z

CC @gregtatum @echeran: can you see what's going on with Cargo.lock?

utils/litemap/src/map.rs

jira-pull-request-webhook · 2021-02-22T21:16:12Z

Notice: the branch changed across the force-push!

Cargo.lock is different
utils/litemap/src/map.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

zbraniecki

lgtm!
Please, file a follow up to add serde support.

Manishearth · 2021-02-22T21:40:41Z

Will do once this merges -- @echeran know what to do about the Cargo.lock issues in the tests?

jira-pull-request-webhook · 2021-02-22T21:41:13Z

Notice: the branch changed across the force-push!

utils/litemap/src/map.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

Manishearth · 2021-02-22T21:41:14Z

Wait, no, that fixed itself somehow

Manishearth · 2021-02-22T23:00:37Z

Needs final r+ to merge

Manishearth · 2021-02-23T01:00:17Z

#500

Manishearth requested a review from a team as a code owner February 17, 2021 01:50

Manishearth requested review from echeran and sffc February 17, 2021 01:50

zbraniecki requested changes Feb 17, 2021

View reviewed changes

utils/terrain/src/map.rs Outdated Show resolved Hide resolved

utils/terrain/src/map.rs Outdated Show resolved Hide resolved

utils/terrain/src/map.rs Outdated Show resolved Hide resolved

utils/terrain/src/map.rs Outdated Show resolved Hide resolved

echeran previously approved these changes Feb 17, 2021

View reviewed changes

sffc reviewed Feb 17, 2021

View reviewed changes

Manishearth dismissed echeran’s stale review via e3c1f6d February 17, 2021 16:47

Manishearth force-pushed the terrain branch from 6607787 to e3c1f6d Compare February 17, 2021 16:47

Add terrain crate

8f2a5ac

Manishearth force-pushed the terrain branch from e3c1f6d to 8f2a5ac Compare February 17, 2021 16:59

Add doctests

88cb160

zbraniecki reviewed Feb 17, 2021

View reviewed changes

utils/terrain/Cargo.toml Outdated Show resolved Hide resolved

utils/terrain/src/lib.rs Outdated Show resolved Hide resolved

utils/terrain/src/map.rs Outdated Show resolved Hide resolved

utils/terrain/src/map.rs Outdated Show resolved Hide resolved

Add license header, Cargo manifest keys

3d98646

zbraniecki previously approved these changes Feb 18, 2021

View reviewed changes

sffc requested changes Feb 18, 2021

View reviewed changes

More review fixes

03e006c

Manishearth dismissed zbraniecki’s stale review via 03e006c February 18, 2021 01:18

Manishearth requested a review from sffc February 18, 2021 01:18

sffc requested changes Feb 18, 2021

View reviewed changes

Manishearth added 2 commits February 18, 2021 10:21

Run rustfmt

d6e275d

Add simple readme

0880588

Manishearth changed the title ~~Add terrain crate (TupleVecMap)~~ Add litemap crate (TupleVecMap) Feb 18, 2021

Manishearth requested a review from sffc February 18, 2021 18:41

zbraniecki previously approved these changes Feb 18, 2021

View reviewed changes

sffc previously approved these changes Feb 19, 2021

View reviewed changes

nordzilla reviewed Feb 22, 2021

View reviewed changes

utils/litemap/src/map.rs Show resolved Hide resolved

Manishearth added 2 commits February 22, 2021 12:29

Rename to litemap

483586a

VecMap -> LiteMap

40663dd

Manishearth dismissed stale reviews from sffc and zbraniecki via 2a5cd63 February 22, 2021 21:16

Manishearth force-pushed the terrain branch from 0edc68a to 2a5cd63 Compare February 22, 2021 21:16

zbraniecki previously approved these changes Feb 22, 2021

View reviewed changes

Add K: Borrow<Q>

c9def58

Manishearth dismissed zbraniecki’s stale review via c9def58 February 22, 2021 21:41

Manishearth force-pushed the terrain branch from 2a5cd63 to c9def58 Compare February 22, 2021 21:41

Manishearth requested review from zbraniecki and sffc February 22, 2021 21:41

zbraniecki approved these changes Feb 22, 2021

View reviewed changes

Manishearth merged commit fdd3a0b into unicode-org:master Feb 23, 2021

Manishearth deleted the terrain branch February 23, 2021 00:59

Manishearth mentioned this pull request Feb 23, 2021

Add serde support to litemap #500

Closed

sffc mentioned this pull request Apr 3, 2021

Make standard library dependencies pluggable #3

Closed

zbraniecki mentioned this pull request Apr 14, 2021

ICU4X 0.2 #239

Closed

16 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add litemap crate (TupleVecMap) #496

Add litemap crate (TupleVecMap) #496

Manishearth commented Feb 17, 2021 •

edited

Loading

echeran left a comment

jira-pull-request-webhook bot commented Feb 17, 2021

jira-pull-request-webhook bot commented Feb 17, 2021

coveralls commented Feb 17, 2021 •

edited

Loading

codecov-io commented Feb 17, 2021

Manishearth commented Feb 17, 2021

zbraniecki left a comment

sffc Feb 18, 2021

sffc left a comment

Manishearth commented Feb 18, 2021

sffc commented Feb 19, 2021

jira-pull-request-webhook bot commented Feb 22, 2021

zbraniecki left a comment

Manishearth commented Feb 22, 2021

jira-pull-request-webhook bot commented Feb 22, 2021

Manishearth commented Feb 22, 2021

Manishearth commented Feb 22, 2021

Manishearth commented Feb 23, 2021

Add litemap crate (TupleVecMap) #496

Add litemap crate (TupleVecMap) #496

Conversation

Manishearth commented Feb 17, 2021 • edited Loading

echeran left a comment

Choose a reason for hiding this comment

jira-pull-request-webhook bot commented Feb 17, 2021

jira-pull-request-webhook bot commented Feb 17, 2021

coveralls commented Feb 17, 2021 • edited Loading

Pull Request Test Coverage Report for Build 040c69ba2da56a7a79fec9f291bc5081b7b4e63b-PR-496

💛 - Coveralls

codecov-io commented Feb 17, 2021

Codecov Report

Manishearth commented Feb 17, 2021

zbraniecki left a comment

Choose a reason for hiding this comment

sffc Feb 18, 2021

Choose a reason for hiding this comment

sffc left a comment

Choose a reason for hiding this comment

Manishearth commented Feb 18, 2021

sffc commented Feb 19, 2021

jira-pull-request-webhook bot commented Feb 22, 2021

zbraniecki left a comment

Choose a reason for hiding this comment

Manishearth commented Feb 22, 2021

jira-pull-request-webhook bot commented Feb 22, 2021

Manishearth commented Feb 22, 2021

Manishearth commented Feb 22, 2021

Manishearth commented Feb 23, 2021

Manishearth commented Feb 17, 2021 •

edited

Loading

coveralls commented Feb 17, 2021 •

edited

Loading