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

Performance fixes. #1654

Merged
merged 4 commits into from
Feb 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/memory/arena/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ go_library(
srcs = [
"arena.go",
],
cdeps = ["//core/memory/arena/cc"],
cdeps = ["//core/memory/arena/tracking_arena"],
cgo = True,
clinkopts = [], # keep
importpath = "github.com/google/gapid/core/memory/arena",
Expand Down
13 changes: 13 additions & 0 deletions core/memory/arena/cc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,16 @@ cc_library(
copts = cc_copts(),
visibility = ["//visibility:public"],
)

cc_library(
name = "tracking_arena",
srcs = glob(
["*.cpp"],
),
deps = [
"//core/cc",
],
hdrs = glob(["*.h"]),
copts = cc_copts() + ["-DTRACK_ALLOCATIONS"],
visibility = ["//visibility:public"],
)
20 changes: 20 additions & 0 deletions core/memory/arena/cc/arena.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,46 +44,66 @@ namespace core {
Arena::Arena() {}

Arena::~Arena() {
#ifdef TRACK_ALLOCATIONS
for (void* ptr : allocations) {
::free(ptr);
}
allocations.clear();
#endif
}

void* Arena::allocate(uint32_t size, uint32_t align) {
void* ptr = malloc(size); // TODO: alignment
#ifdef TRACK_ALLOCATIONS
allocations.insert(ptr);
#endif
return ptr;
}

void* Arena::reallocate(void* ptr, uint32_t size, uint32_t align) {
GAPID_ASSERT_MSG(this->owns(ptr), "ptr: %p", ptr);
void* newptr = realloc(ptr, size); // TODO: alignment
#ifdef TRACK_ALLOCATIONS
allocations.erase(ptr);
allocations.insert(newptr);
#endif
return newptr;
}

void Arena::free(void* ptr) {
GAPID_ASSERT_MSG(this->owns(ptr), "ptr: %p", ptr);
#ifdef TRACK_ALLOCATIONS
allocations.erase(ptr);
#endif
::free(ptr);
}

bool Arena::owns(void* ptr) {
#ifdef TRACK_ALLOCATIONS
return allocations.count(ptr) == 1;
#else
return true;
#endif
}

size_t Arena::num_allocations() const {
#ifdef TRACK_ALLOCATIONS
return allocations.size();
#else
return 0;
#endif
}

size_t Arena::num_bytes_allocated() const {
#ifdef TRACK_ALLOCATIONS
size_t bytes = 0;
for (void* ptr : allocations) {
bytes += alloc_size(ptr);
}
return bytes;
#else
return 0;
#endif
}

} // namespace core
Expand Down
14 changes: 10 additions & 4 deletions gapil/compiler/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ import (
// If we end up with lots of insertions/deletions, this will prevent linear search

func (c *C) defineMapType(t *semantic.Map) {

if ((minMapSize & (minMapSize - 1)) != 0) ||
((mapGrowMultiplier & (mapGrowMultiplier - 1)) != 0) {
fail("Map size must be a power of 2")
}

mapPtrTy := c.T.target[t].(codegen.Pointer)
mapStrTy := mapPtrTy.Element.(*codegen.Struct)
keyTy := c.T.Target(t.KeyType)
Expand Down Expand Up @@ -219,7 +225,7 @@ func (c *C) buildMapType(t *semantic.Map) {
capacity := m.Index(0, MapCapacity).Load()
elements := m.Index(0, MapElements).Load()
s.ForN(capacity, func(it *codegen.Value) *codegen.Value {
check := s.Rem(s.Add(h, it), capacity)
check := s.And(s.Add(h, it), s.Sub(capacity, s.Scalar(u64(1))))
valid := elements.Index(check, "used").Load()
s.If(c.equal(s, valid, s.Scalar(mapElementEmpty)), func() {
s.Return(s.Scalar(false))
Expand Down Expand Up @@ -251,7 +257,7 @@ func (c *C) buildMapType(t *semantic.Map) {
h := c.hashValue(s, t.KeyType, k)
// Search for existing
s.ForN(capacity, func(it *codegen.Value) *codegen.Value {
check := s.Rem(s.Add(h, it), capacity)
check := s.And(s.Add(h, it), s.Sub(capacity, s.Scalar(u64(1))))
valid := elements.Index(check, "used").Load()
s.If(c.equal(s, valid, s.Scalar(mapElementFull)), func() {
found := c.equal(s, elements.Index(check, "k").Load(), k)
Expand All @@ -273,7 +279,7 @@ func (c *C) buildMapType(t *semantic.Map) {
getStorageBucket := func(h, table, tablesize *codegen.Value) *codegen.Value {
newBucket := s.Local("newBucket", u64Type)
s.ForN(tablesize, func(it *codegen.Value) *codegen.Value {
check := s.Rem(s.Add(h, it), tablesize).SetName("hash_bucket")
check := s.And(s.Add(h, it), s.Sub(tablesize, s.Scalar(u64(1)))).SetName("hash_bucket")
newBucket.Store(check)
valid := table.Index(check, "used").Load()
notFound := c.equal(s, valid, s.Scalar(mapElementFull))
Expand Down Expand Up @@ -364,7 +370,7 @@ func (c *C) buildMapType(t *semantic.Map) {
elements := m.Index(0, MapElements).Load()
// Search for existing
s.ForN(capacity, func(it *codegen.Value) *codegen.Value {
check := s.Rem(s.Add(h, it), capacity)
check := s.And(s.Add(h, it), s.Sub(capacity, s.Scalar(u64(1))))
valid := elements.Index(check, "used").Load()
s.If(c.equal(s, valid, s.Scalar(mapElementFull)), func() {
found := c.equal(s, elements.Index(check, "k").Load(), k)
Expand Down
18 changes: 15 additions & 3 deletions gapil/runtime/cc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,25 @@ cc_library(
hdrs = glob(["*.h"]),
copts = cc_copts(),
visibility = ["//visibility:public"],
)

cc_library(
name = "cc",
srcs = [
"runtime.cpp",
"string.cpp",
],
hdrs = glob(["*.inc"]),
copts = cc_copts(),
visibility = ["//visibility:public"],
deps = [
":headers",
"//core/memory/arena/cc",
],
)

cc_library(
name = "cc",
name = "test_cc",
srcs = [
"runtime.cpp",
"string.cpp",
Expand All @@ -35,7 +47,7 @@ cc_library(
visibility = ["//visibility:public"],
deps = [
":headers",
"//core/memory/arena/cc",
"//core/memory/arena/cc:tracking_arena",
],
)

Expand All @@ -53,7 +65,7 @@ cc_test(
"//conditions:default": [],
}),
deps = [
":cc",
":test_cc",
"@gtest//:gtest_main",
]
)
9 changes: 6 additions & 3 deletions gapil/runtime/cc/map.inc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@

#include <unordered_map>

static_assert((GAPIL_MIN_MAP_SIZE & (GAPIL_MIN_MAP_SIZE - 1)) == 0, "Map size must be a power of 2");
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth putting a similar assert in the compiler.

static_assert((GAPIL_MAP_GROW_MULTIPLIER & (GAPIL_MAP_GROW_MULTIPLIER - 1)) == 0, "Map size must be a power of 2");

namespace gapil {

template<typename K, typename V>
Expand Down Expand Up @@ -109,7 +112,7 @@ V* Map<K, V>::Allocation::index(K key, bool insert) {

for (uint64_t i = 0; i < capacity; ++i) {
bool leave = false;
uint64_t lookup_pos = (hash + i) % capacity;
uint64_t lookup_pos = (hash + i) & (capacity - 1);
switch(elems[lookup_pos].used) {
case GAPIL_MAP_ELEMENT_EMPTY:
leave = true;
Expand All @@ -131,7 +134,7 @@ V* Map<K, V>::Allocation::index(K key, bool insert) {
auto storageBucket = [&](uint64_t h) {
auto elems = els();
for (uint64_t i = 0; i < capacity; ++i) {
uint64_t x = (h + i) % capacity;
uint64_t x = (h + i) & (capacity - 1);
if (elems[x].used != GAPIL_MAP_ELEMENT_FULL) {
return x;
}
Expand Down Expand Up @@ -201,7 +204,7 @@ void Map<K, V>::Allocation::remove(K key) {
auto elems = els();

for (uint64_t i = 0; i < capacity; ++i) {
uint64_t lookup_pos = (hash + i) % capacity;
uint64_t lookup_pos = (hash + i) & (capacity - 1);
switch(elems[lookup_pos].used) {
case GAPIL_MAP_ELEMENT_EMPTY:
return;
Expand Down
5 changes: 5 additions & 0 deletions gapil/runtime/cc/runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
#include <stdlib.h>
#include <stdarg.h>

#if TARGET_OS == GAPID_OS_ANDROID
// for snprintf
#include <cstdio>
#endif

#define __STDC_FORMAT_MACROS
#include <inttypes.h>

Expand Down