-
Notifications
You must be signed in to change notification settings - Fork 30
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
support for custom allocator in ByteBuffer #1845
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/* This file is part of magic-sdk, an sdk for the open source programming language magic. | ||
* | ||
* Copyright (C) 2016 magic-lang | ||
* | ||
* This software may be modified and distributed under the terms | ||
* of the MIT license. See the LICENSE file for details. | ||
*/ | ||
|
||
AbstractAllocator: abstract class { | ||
allocate: abstract func (size: SizeT) -> Void* | ||
deallocate: abstract func (pointer: Void*) | ||
} | ||
|
||
MallocAllocator: class extends AbstractAllocator { | ||
init: func | ||
allocate: override func (size: SizeT) -> Void* { malloc(size) } | ||
deallocate: override func (pointer: Void*) { memfree(pointer) } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
*/ | ||
|
||
use collections | ||
import Allocator | ||
import ReferenceCounter | ||
import Debug | ||
|
||
|
@@ -15,21 +16,24 @@ ByteBuffer: class { | |
_size: Int | ||
_referenceCount: ReferenceCounter | ||
_ownsMemory: Bool | ||
_allocator: AbstractAllocator = null | ||
pointer ::= this _pointer | ||
size ::= this _size | ||
referenceCount ::= this _referenceCount | ||
|
||
init: func (=_pointer, =_size, ownsMemory := false) { | ||
init: func (=_pointer, =_size, ownsMemory := false, allocator := null as AbstractAllocator) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For some reason
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, rock does not handle this correctly: ooc-lang/rock#990 |
||
this _referenceCount = ReferenceCounter new(this) | ||
this _ownsMemory = ownsMemory | ||
this _allocator = allocator ?? MallocAllocator new() as AbstractAllocator | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cast is needed to prevent warning |
||
} | ||
free: override func { | ||
if (this _referenceCount != null) | ||
this _referenceCount free() | ||
this _referenceCount = null | ||
if (this _ownsMemory) | ||
memfree(this _pointer) | ||
this _allocator deallocate(this _pointer) | ||
this _pointer = null | ||
this _allocator free() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the allocator really be owned by the buffer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess that's easiest for now. Allocator could also have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's quite natural for the buffer to take ownership of the allocator. @emilwestergren can you think of a case were this would be undesirable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not uncommon to have an allocator as a singleton for example. In most cases you will maybe have a few different allocators, and to allocate and free these for every ByteBuffer seems like a waste. We could for example have a few static standard allocators in the ByteBuffer or Allocator class. buffer := ByteBuffer new(size, Allocator malloc) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes and a possibility to override the default allocator, as my point here was to allow custom memory allocation for byte buffers in raster image classes. This is currently not possible, but for the sake of code quality I happily sacrifice my own personal short-sighted goals. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point |
||
super() | ||
} | ||
zero: func ~whole { this memset(0) } | ||
|
@@ -50,7 +54,7 @@ ByteBuffer: class { | |
copyTo: func (other: This, start: Int, destination: Int, length: Int) { | ||
memcpy(other pointer + destination, this pointer + start, length) | ||
} | ||
new: static func ~size (size: Int) -> This { _RecyclableByteBuffer new(size) } | ||
new: static func ~size (size: Int, allocator: AbstractAllocator = null) -> This { _RecyclableByteBuffer new(size, allocator) } | ||
new: static func ~recover (pointer: Byte*, size: Int, recover: Func (This) -> Bool) -> This { | ||
_RecoverableByteBuffer new(pointer, size, recover) | ||
} | ||
|
@@ -86,7 +90,7 @@ _RecoverableByteBuffer: class extends ByteBuffer { | |
} | ||
|
||
_RecyclableByteBuffer: class extends ByteBuffer { | ||
init: func (pointer: Byte*, size: Int) { super(pointer, size, true) } | ||
init: func (pointer: Byte*, size: Int, allocator: AbstractAllocator = null) { super(pointer, size, true, allocator) } | ||
_forceFree: func { | ||
this _size = 0 | ||
this free() | ||
|
@@ -111,7 +115,7 @@ _RecyclableByteBuffer: class extends ByteBuffer { | |
_smallRecycleBin := static VectorList<This> new() | ||
_mediumRecycleBin := static VectorList<This> new() | ||
_largeRecycleBin := static VectorList<This> new() | ||
new: static func ~fromSize (size: Int) -> This { | ||
new: static func ~fromSize (size: Int, allocator: AbstractAllocator = null) -> This { | ||
buffer: This = null | ||
bin := This _getBin(size) | ||
This _lock lock() | ||
|
@@ -123,7 +127,8 @@ _RecyclableByteBuffer: class extends ByteBuffer { | |
} | ||
This _lock unlock() | ||
version(debugByteBuffer) { if (buffer == null) Debug print("No RecyclableByteBuffer available in the bin; allocating a new one") } | ||
buffer == null ? This new(malloc(size), size) : buffer | ||
allocator = allocator ?? MallocAllocator new() as AbstractAllocator | ||
buffer == null ? This new(allocator allocate(size) as Byte*, size, allocator) : buffer | ||
} | ||
_getBin: static func (size: Int) -> VectorList<This> { | ||
size < 10000 ? This _smallRecycleBin : (size < 100000 ? This _mediumRecycleBin : This _largeRecycleBin) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,18 @@ | |
use base | ||
use unit | ||
|
||
CustomAllocator: class extends AbstractAllocator { | ||
_allocCount := 0 | ||
init: func | ||
allocate: override func (size: SizeT) -> Void* { | ||
this _allocCount += 1 | ||
malloc(size) | ||
} | ||
deallocate: override func (pointer: Void*) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
memfree(pointer) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. |
||
} | ||
} | ||
|
||
ByteBufferTest: class extends Fixture { | ||
init: func { | ||
super("ByteBuffer") | ||
|
@@ -78,6 +90,14 @@ ByteBufferTest: class extends Fixture { | |
expect(buffer pointer[63] as Int, is equal to(63)) | ||
buffer referenceCount decrease() | ||
}) | ||
this add("custom alloc", This _testCustomAlloc) | ||
} | ||
_testCustomAlloc: static func { | ||
allocator := CustomAllocator new() | ||
expect(allocator _allocCount, is equal to(0)) | ||
buffer := ByteBuffer new(128, allocator) | ||
expect(allocator _allocCount, is equal to(1)) | ||
buffer free() | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another way is to simply have a concrete
Allocator
class where you can supply allocate/deallocate closures. That way you don't have to inherit the class for every single allocator, only if you need to add more behaviour. Also, then we can add static default allocators in this class.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. I agree that allocating allocators for every object seems unnecessary.
I shall add default allocators now to prevent this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also use template classes to create composable allocators (I'm thinking about Andrei Alexandrescu's design of the D std.allocator), although they are quite broken right now.
By the way, another nice way to expose allocators to any ooc code would be to add a global
current_allocator
variable and use it to allocate and free (inObject alloc
andObject free
) although that means that you then have to keep track of what variables were allocated with what allocator etc.With such a design, I expect you would use a single well tuned allocator for most allocations and switch allocators in specific places (for example,
current_allocator = SomePoolAllocator; /* allocate thousands of small objects here, use them, free them */; current_allocator = SystemWideAllocator
).EDIT: Also, you could then do stuff like that:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Shamanas
use template classes
⬆️ that was the first thing I though of, but I gave up after fighting some compiler errors :/ This here is not perfect, but suits my needs for now. You have some nice ideas here, I think we may revisit this someday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebastianbaginski
Yeah template classes definitely do need a whole lot of patching, althoug hthe only error I can currently think of right now (which is pretty big itself) is that modules that define them need to import all modules of the types they are instanciated with.