From 67e6c2b4f59e760b60d6eaf3bacb31c5d844e2a0 Mon Sep 17 00:00:00 2001 From: Tiyash Basu Date: Fri, 24 May 2019 14:17:22 +0200 Subject: [PATCH] Change storage engine getter to return via delegate This improves performance by reducing by one the number of times the value(s) gets copied before being returned. The requests affected in this change are Get, GetChannelSize, GetChannels, GetNumConnections, GetResponsibleRange, GetSize. Fixes #144 --- relnotes/GetterRequests.migration.md | 11 +++++++ src/dhtproto/node/request/Get.d | 11 +++++-- src/dhtproto/node/request/GetChannelSize.d | 21 +++++++----- src/dhtproto/node/request/GetChannels.d | 17 ++++++---- src/dhtproto/node/request/GetNumConnections.d | 30 ++++++++++------- .../node/request/GetResponsibleRange.d | 21 +++++++----- src/dhtproto/node/request/GetSize.d | 33 +++++++++++-------- src/fakedht/request/Get.d | 15 +++++---- src/fakedht/request/GetChannelSize.d | 6 ++-- src/fakedht/request/GetChannels.d | 13 +++++--- src/fakedht/request/GetNumConnections.d | 11 ++++--- src/fakedht/request/GetResponsibleRange.d | 8 ++--- src/fakedht/request/GetSize.d | 12 ++++--- 13 files changed, 130 insertions(+), 79 deletions(-) create mode 100644 relnotes/GetterRequests.migration.md diff --git a/relnotes/GetterRequests.migration.md b/relnotes/GetterRequests.migration.md new file mode 100644 index 00000000..0837bbd2 --- /dev/null +++ b/relnotes/GetterRequests.migration.md @@ -0,0 +1,11 @@ +### Request getters return values via delegates + +`dhtproto.node.request.Get`, `dhtproto.node.request.GetChannelSize`, +`dhtproto.node.request.GetChannels`, `dhtproto.node.request.GetNumConnections`, +`dhtproto.node.request.GetResponsibleRange`, `dhtproto.node.request.GetSize`, +`fakedht.request.Get`, `fakedht.request.GetChannelSize`, +`fakedht.request.GetChannels`, `fakedht.request.GetNumConnections`, +`fakedht.request.GetResponsibleRange`, `fakedht.request.GetSize` + +This improves performance by reducing by one the number of times the +value(s) gets copied before being returned. diff --git a/src/dhtproto/node/request/Get.d b/src/dhtproto/node/request/Get.d index 67f902e7..ee3a10d0 100644 --- a/src/dhtproto/node/request/Get.d +++ b/src/dhtproto/node/request/Get.d @@ -91,7 +91,12 @@ public abstract scope class Get : SingleKey cstring key ) { this.writer.write(DhtConst.Status.E.Ok); - this.writer.writeArray(this.getValue(channel_name, key)); + this.getValue(channel_name, key, + ( const(void)[] value ) + { + this.writer.writeArray(value); + } + ); } /*************************************************************************** @@ -102,11 +107,13 @@ public abstract scope class Get : SingleKey Params: channel_name = name of channel to query key = key of record to find + value_getter_dg = The delegate that is called with the value. Returns: value of queried record, empty array if not found ***************************************************************************/ - abstract protected Const!(void)[] getValue ( cstring channel_name, cstring key ); + abstract protected void getValue ( cstring channel_name, cstring key, + scope void delegate ( const(void)[] ) value_getter_dg ); } diff --git a/src/dhtproto/node/request/GetChannelSize.d b/src/dhtproto/node/request/GetChannelSize.d index 9ad8872e..647333af 100644 --- a/src/dhtproto/node/request/GetChannelSize.d +++ b/src/dhtproto/node/request/GetChannelSize.d @@ -78,13 +78,16 @@ public abstract scope class GetChannelSize : SingleChannel { this.writer.write(DhtConst.Status.E.Ok); - auto data = this.getChannelData(channel_name); - - // TODO: is there a need to send the addr/port? surely the client knows this anyway? - this.writer.writeArray(data.address); - this.writer.write(data.port); - this.writer.write(data.records); - this.writer.write(data.bytes); + this.getChannelData(channel_name, + ( ChannelSizeData data ) + { + // TODO: is there a need to send the addr/port? + // surely the client knows this anyway? + this.writer.writeArray(data.address); + this.writer.write(data.port); + this.writer.write(data.records); + this.writer.write(data.bytes); + }); } /*************************************************************************** @@ -94,8 +97,10 @@ public abstract scope class GetChannelSize : SingleChannel Params: channel_name = name of channel to be queried + value_getter_dg = The delegate that is called with the channel data. ***************************************************************************/ - abstract protected ChannelSizeData getChannelData ( cstring channel_name ); + abstract protected void getChannelData ( cstring channel_name, + scope void delegate ( ChannelSizeData ) value_getter_dg ); } diff --git a/src/dhtproto/node/request/GetChannels.d b/src/dhtproto/node/request/GetChannels.d index ae613e60..df175890 100644 --- a/src/dhtproto/node/request/GetChannels.d +++ b/src/dhtproto/node/request/GetChannels.d @@ -66,10 +66,11 @@ public abstract scope class GetChannels : DhtCommand final override protected void handleRequest ( ) { this.writer.write(DhtConst.Status.E.Ok); - foreach (id; this.getChannelsIds()) - { - this.writer.writeArray(id); - } + this.getChannelsIds( + ( const(void)[] id ) + { + this.writer.writeArray(id); + }); this.writer.writeArray(""); // End of list } @@ -77,10 +78,12 @@ public abstract scope class GetChannels : DhtCommand Must return list of all channels stored in this node. - Returns: - list of channel names + Params: + value_getter_dg = The delegate that is called with the list of + channel names. ***************************************************************************/ - abstract protected Const!(char[][]) getChannelsIds ( ); + abstract protected void getChannelsIds ( + scope void delegate ( const(void)[] ) value_getter_dg ); } diff --git a/src/dhtproto/node/request/GetNumConnections.d b/src/dhtproto/node/request/GetNumConnections.d index 145d4ae3..0815db33 100644 --- a/src/dhtproto/node/request/GetNumConnections.d +++ b/src/dhtproto/node/request/GetNumConnections.d @@ -78,24 +78,30 @@ public abstract scope class GetNumConnections : DhtCommand final override protected void handleRequest ( ) { - auto data = this.getConnectionsData(); - - this.writer.write(DhtConst.Status.E.Ok); - - // TODO: is there a need to send the addr/port? surely the client knows this anyway? - this.writer.writeArray(data.address); - this.writer.write(data.port); - this.writer.write(data.num_conns); + this.getConnectionsData( + ( NumConnectionsData data ) + { + this.writer.write(DhtConst.Status.E.Ok); + + // TODO: is there a need to send the addr/port? + // surely the client knows this anyway? + this.writer.writeArray(data.address); + this.writer.write(data.port); + this.writer.write(data.num_conns); + } + ); } /*************************************************************************** - Must return total num_conns of established connections to this node. + Gets the total num_conns of established connections to this node. - Returns: - metadata that includes number of established connections + Params: + value_getter_dg = The delegate that is called with the metadata + that includes number of established connections. ***************************************************************************/ - abstract protected NumConnectionsData getConnectionsData ( ); + abstract protected void getConnectionsData ( + scope void delegate ( NumConnectionsData ) value_getter_dg ); } diff --git a/src/dhtproto/node/request/GetResponsibleRange.d b/src/dhtproto/node/request/GetResponsibleRange.d index bd9a66bf..1b0987e3 100644 --- a/src/dhtproto/node/request/GetResponsibleRange.d +++ b/src/dhtproto/node/request/GetResponsibleRange.d @@ -63,23 +63,26 @@ public abstract scope class GetResponsibleRange : DhtCommand final override protected void handleRequest ( ) { - this.writer.write(DhtConst.Status.E.Ok); - hash_t min, max; - this.getRangeLimits(min, max); - this.writer.write(min); - this.writer.write(max); + this.getRangeLimits( + ( hash_t min, hash_t max ) + { + this.writer.write(DhtConst.Status.E.Ok); + this.writer.write(min); + this.writer.write(max); + }); } /*************************************************************************** - Must return minimum and maximum allowed hash value this node + Get the minimum and maximum allowed hash value this node is responsible for. Params: - min = minimal allowed hash - max = maximal allowed hash + value_getter_dg = The delegate that is called with the minimum and + the maximum allowed hashes. ***************************************************************************/ - abstract protected void getRangeLimits ( out hash_t min, out hash_t max ); + abstract protected void getRangeLimits ( + scope void delegate ( hash_t min, hash_t max ) value_getter_dg ); } diff --git a/src/dhtproto/node/request/GetSize.d b/src/dhtproto/node/request/GetSize.d index 6e4deaea..688f5a76 100644 --- a/src/dhtproto/node/request/GetSize.d +++ b/src/dhtproto/node/request/GetSize.d @@ -51,7 +51,7 @@ public abstract scope class GetSize : DhtCommand /*************************************************************************** - Payload structs that holds requested metadata + Payload structs that holds requested metadata ***************************************************************************/ @@ -79,25 +79,30 @@ public abstract scope class GetSize : DhtCommand final override protected void handleRequest ( ) { - auto data = this.getSizeData(); - - this.writer.write(DhtConst.Status.E.Ok); - - // TODO: is there a need to send the addr/port? surely the client knows this anyway? - this.writer.writeArray(data.address); - this.writer.write(data.port); - this.writer.write(data.records); - this.writer.write(data.bytes); + this.getSizeData( + ( SizeData data ) + { + this.writer.write(DhtConst.Status.E.Ok); + + // TODO: is there a need to send the addr/port? + // surely the client knows this anyway? + this.writer.writeArray(data.address); + this.writer.write(data.port); + this.writer.write(data.records); + this.writer.write(data.bytes); + }); } /*************************************************************************** - Must return aggregated size of all channels. + Gets the aggregated size of all channels. - Returns: - metadata that includes the size + Params: + value_getter_dg = The delegate that is called with the metadata + that includes the size. ***************************************************************************/ - abstract protected SizeData getSizeData ( ); + abstract protected void getSizeData ( + scope void delegate ( SizeData ) value_getter_dg ); } diff --git a/src/fakedht/request/Get.d b/src/fakedht/request/Get.d index ea457c37..ba61732d 100644 --- a/src/fakedht/request/Get.d +++ b/src/fakedht/request/Get.d @@ -50,17 +50,18 @@ public scope class Get : Protocol.Get Params: channel_name = name of channel to query key = key of record to find - - Returns: - value of queried record, empty array if not found + value_getter_dg = The delegate that is called with the value. ***************************************************************************/ - override protected Const!(void)[] getValue ( cstring channel_name, cstring key ) + override protected void getValue ( cstring channel_name, cstring key, + scope void delegate ( const(void)[] ) value_getter_dg ) { auto channel = global_storage.get(channel_name); - if (channel is null) - return null; - return channel.get(key); + + if (channel !is null) + { + value_getter_dg(channel.get(key)); + } } } diff --git a/src/fakedht/request/GetChannelSize.d b/src/fakedht/request/GetChannelSize.d index 63623441..89c6557f 100644 --- a/src/fakedht/request/GetChannelSize.d +++ b/src/fakedht/request/GetChannelSize.d @@ -49,15 +49,17 @@ public scope class GetChannelSize : Protocol.GetChannelSize Params: channel_name = name of channel to be queried + value_getter_dg = The delegate that is called with the channel data. ***************************************************************************/ - override protected ChannelSizeData getChannelData ( cstring channel_name ) + override protected void getChannelData ( cstring channel_name, + scope void delegate ( ChannelSizeData ) value_getter_dg ) { ChannelSizeData result; auto channel = global_storage.get(channel_name); if (channel !is null) channel.countSize(result.records, result.bytes); - return result; + value_getter_dg(result); } } diff --git a/src/fakedht/request/GetChannels.d b/src/fakedht/request/GetChannels.d index b90fcd49..bd9f4392 100644 --- a/src/fakedht/request/GetChannels.d +++ b/src/fakedht/request/GetChannels.d @@ -46,13 +46,18 @@ public scope class GetChannels : Protocol.GetChannels Must return list of all channels stored in this node. - Returns: - list of channel names + Params: + value_getter_dg = The delegate that is called with the list of + channel names. ***************************************************************************/ - override protected Const!(char[][]) getChannelsIds ( ) + override protected void getChannelsIds ( + scope void delegate ( const(void)[] ) value_getter_dg ) { - return global_storage.getChannelList(); + foreach (ref id; global_storage.getChannelList()) + { + value_getter_dg(id); + } } } diff --git a/src/fakedht/request/GetNumConnections.d b/src/fakedht/request/GetNumConnections.d index f5da110a..a09655c9 100644 --- a/src/fakedht/request/GetNumConnections.d +++ b/src/fakedht/request/GetNumConnections.d @@ -42,17 +42,18 @@ public scope class GetNumConnections : Protocol.GetNumConnections /*************************************************************************** - Must return total num_conns of established connections to this node. + Gets the total num_conns of established connections to this node. - Returns: - metadata that includes number of established connections + Params: + value_getter_dg = The delegate that is called with the metadata + that includes number of established connections. ***************************************************************************/ - override protected NumConnectionsData getConnectionsData ( ) + override protected void getConnectionsData ( + scope void delegate ( NumConnectionsData ) /* value_getter_dg */ ) { enforce(false, "GetNumConnections is not supported by the fake DHT node"); - return NumConnectionsData.init; } } diff --git a/src/fakedht/request/GetResponsibleRange.d b/src/fakedht/request/GetResponsibleRange.d index b2753859..26ac3fbe 100644 --- a/src/fakedht/request/GetResponsibleRange.d +++ b/src/fakedht/request/GetResponsibleRange.d @@ -41,7 +41,7 @@ public scope class GetResponsibleRange : Protocol.GetResponsibleRange /*************************************************************************** - Must return minimum and maximum allowed hash value this node + Get the return minimum and maximum allowed hash value this node is responsible for. Params: @@ -50,9 +50,9 @@ public scope class GetResponsibleRange : Protocol.GetResponsibleRange ***************************************************************************/ - override protected void getRangeLimits ( out hash_t min, out hash_t max ) + override protected void getRangeLimits ( + scope void delegate ( hash_t min, hash_t max ) value_getter_dg ) { - min = hash_t.min; - max = hash_t.max; + value_getter_dg(hash_t.min, hash_t.max); } } diff --git a/src/fakedht/request/GetSize.d b/src/fakedht/request/GetSize.d index 89edb4f0..d1efd5b3 100644 --- a/src/fakedht/request/GetSize.d +++ b/src/fakedht/request/GetSize.d @@ -42,14 +42,16 @@ public scope class GetSize : Protocol.GetSize /*************************************************************************** - Must return aggregated size of all channels. + Gets the aggregated size of all channels. - Returns: - metadata that includes the size + Params: + value_getter_dg = The delegate that is called with the metadata + that includes the size. ***************************************************************************/ - override protected SizeData getSizeData ( ) + override protected void getSizeData ( + scope void delegate ( SizeData ) value_getter_dg ) { SizeData result; auto channels = global_storage.getChannelList(); @@ -62,6 +64,6 @@ public scope class GetSize : Protocol.GetSize result.bytes += bytes; } - return result; + value_getter_dg(result); } }