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

Add test case for map API #141

Merged
merged 1 commit into from
Jun 4, 2023
Merged

Add test case for map API #141

merged 1 commit into from
Jun 4, 2023

Conversation

EagleTw
Copy link
Collaborator

@EagleTw EagleTw commented Jun 4, 2023

In preparation for new map implementation

jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv requested changes Jun 4, 2023
View reviewed changes
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the assertions.

jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
@jserv jserv changed the title Add testcase for map Add test case for map API Jun 4, 2023
@EagleTw EagleTw force-pushed the add-test-map branch 3 times, most recently from 517af99 to 5c097fd Compare June 4, 2023 07:47
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv requested changes Jun 4, 2023
View reviewed changes
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the correctness verification, add execution time measurements, so that we can
distinguish from each implementation.

@jserv
Copy link
Contributor

jserv commented Jun 4, 2023

You may check https://github.com/martinus/map_benchmark/tree/master/src/benchmarks for figuring out the available benchmarking on std::map.

@EagleTw EagleTw force-pushed the add-test-map branch 2 times, most recently from 4923eba to 99b9ad1 Compare June 4, 2023 10:21
@EagleTw
Copy link
Collaborator Author

EagleTw commented Jun 4, 2023

I wonder whether it is possible to checkin the master first.
Then, I'll implement time measurements for newer features later.

jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
@EagleTw EagleTw force-pushed the add-test-map branch 3 times, most recently from e01dae8 to 96ad955 Compare June 4, 2023 12:07
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
jserv
jserv reviewed Jun 4, 2023
View reviewed changes
@jserv jserv merged commit ae34225 into sysprog21:master Jun 4, 2023
@jserv
Copy link
Contributor

jserv commented Jun 4, 2023

Thank @ypaskell for contributing!

vestata pushed a commit to vestata/rv32emu that referenced this pull request Jan 24, 2025
@jserv
Merge pull request sysprog21#141 from ypaskell/add-test-map
68986a4 
Add test case for map API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jserv jserv jserv requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@EagleTw @jserv