Skip to content
This repository has been archived by the owner on Aug 13, 2019. It is now read-only.

Use the right sized byte array for large indexes #301

Merged
merged 1 commit into from
Mar 14, 2018

Conversation

gouthamve
Copy link
Collaborator

@gouthamve gouthamve commented Mar 14, 2018

Fixes: prometheus/prometheus#3957

Not happy with this, suggestions welcome. @Bplotka


This change is Reviewable

@gouthamve gouthamve requested review from fabxc and brian-brazil March 14, 2018 07:55
return (*[1 << 32]byte)(unsafe.Pointer(addr))[:sz], nil
}

if sz <= 1<<34 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not always use this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are allocating 16G everytime? Whether or not we are using it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that's an allocation?

@gouthamve gouthamve force-pushed the fix-prom-3957 branch 2 times, most recently from 12a45e6 to 89b0e10 Compare March 14, 2018 09:22
@@ -35,7 +35,7 @@ func mmap(f *os.File, sz int) ([]byte, error) {
return nil, os.NewSyscallError("CloseHandle", err)
}

return (*[1 << 30]byte)(unsafe.Pointer(addr))[:sz], nil
Copy link
Contributor

Choose a reason for hiding this comment

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

sz is just size? Can we name it like that? (maybe just for master)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I have stupid question. I have never used mmap before, but... why don't we cast to just [sz]byte here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking the same thing and tried it but casting to an array requires a constant for the array size as this is evaluated at compile time

  • return (*[sz]byte)(unsafe.Pointer(addr))[:sz], nil = non-constant array bound

Fixes: prometheus/prometheus#3957

Signed-off-by: Goutham Veeramachaneni <cs14btech11014@iith.ac.in>
@fabxc
Copy link
Contributor

fabxc commented Mar 14, 2018

👍

tsdb needs another revendor then

@gouthamve gouthamve merged commit 902e1ff into prometheus-junkyard:master Mar 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants