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

modified freelist_hmap/hashmapGetFreePageIDs function with better performance #219

Closed
wants to merge 2 commits into from

Conversation

nicktming
Copy link

Since forwardMap key is the start point, value is the length starting with the key, then we just sort the key to make the whole array sorted instead of sorting the whole array.

This is my performance test compared with these two algorithms:

  1. N: is the number of forwardMap key
  2. val: is the value of each key
  3. time_used_origin: is the time used of the origin function
  4. time_used_new: is the time used of the improved function
N val time_used_origin time_used_new
10000 1 2.301202ms 2.614463ms
10000 10 18.204936ms 2.942327ms
10000 100 164.167666ms 7.275755ms
10000 1000 1.71088519s 115.790789ms
10000 10000 19.85861009s 1.625219654s
func main() {
	N := int32(10000)
	fm := make(map[pgid]uint64)
	i := int32(0)
	val := int32(0)
	for i = 0; i < N;  {
		val = rand.Int31n(1000)
		fm[pgid(i)] = uint64(val)
		i += val
	}

	f := freelist{
		forwardMap: fm,
	}
	start := time.Now()
	f.hashmapGetFreePageIDs()
	end := time.Now()
	fmt.Printf("origin time:%v\n", end.Sub(start))

	start = time.Now()
	res := f.newHashmapGetFreePageIDs()
	end = time.Now()
	fmt.Printf("new time:%v\n", end.Sub(start))


	if !sort.SliceIsSorted(res, func(i, j int) bool { return res[i] < res[j] }) {
		panic("pgids not sorted")
	}
}

based on the above testing program, when changing N, got the following result compared with two functions

ENV : 8 GB 2133 MHz LPDDR3 / 1.6 GHz 2 Cores Intel Core i5

N time_used_origin time_used_new
100 71.608µs 9.022µs
1000 226.745µs 19.359µs
10000 1.290836ms 77.194µs
100000 15.973611ms 777.995µs
1000000 160.912048ms 5.714576ms
10000000 1.789962277s 99.443407ms

for i := 0; i < int(size); i++ {
m = append(m, start+pgid(i))

keys := make([]pgid, 0, len(f.forwardMap))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
keys := make([]pgid, 0, len(f.forwardMap))
startPageIds := make([]pgid, 0, len(f.forwardMap))

Comment on lines +87 to +88
size, ok := f.forwardMap[start]
if ok {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
size, ok := f.forwardMap[start]
if ok {
if size, ok := f.forwardMap[start]; ok {

@ahrtr
Copy link
Member

ahrtr commented Nov 30, 2022

This PR looks good to me. Please

  1. squash the commit and signoff the final commit;
  2. rebase this PR (Note: we added some github workflows).

cc @ptabor @xiang90

@ahrtr ahrtr added the resurrection The PR makes sense but not responsive. So please anyone feel free to resurrect this PR label Dec 30, 2022
@ahrtr
Copy link
Member

ahrtr commented Dec 30, 2022

Added label resurrection because there is no response from the original author. Please anyone else feel free to resurrect this PR.

@cenkalti
Copy link
Member

Rebased the commits and sent as new PR #419

@ahrtr
Copy link
Member

ahrtr commented Mar 14, 2023

superseded by #419

@ahrtr ahrtr closed this Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resurrection The PR makes sense but not responsive. So please anyone feel free to resurrect this PR
Development

Successfully merging this pull request may close these issues.

3 participants