asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Fix Decoding of byte[] Records
Date Tue, 21 Jun 2016 01:05:55 GMT
abdullah alamoudi has posted comments on this change.

Change subject: Fix Decoding of byte[] Records
......................................................................


Patch Set 2:

(2 comments)

https://asterix-gerrit.ics.uci.edu/#/c/951/2/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/converter/DCPMessageToRecordConverter.java
File asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/input/record/converter/DCPMessageToRecordConverter.java:

Line 42: public class DCPMessageToRecordConverter implements IRecordToRecordWithMetadataAndPKConverter<DCPRequest,
char[]> {
> Is the code style correct?
If by Style, you mean the formatting? then I think so.

If by style you mean interfaces and class design, Till and I have been discussing going over
the interfaces at some point.
For now, this is correct and it means that you give this converter a DCPRequest (DCPRequest
id a dcp message. I don't know why they named the class DCPRequest and I brought it up with
them). The char[] means that the returned record object has a char[] which can be parsed.


Line 116:             bytes.compact();
> A lot of memory copies here?
the SDK use netty byte buffer which id different from java ByteBuffer. the decoder only deals
with ByteBuffers and CharBuffer. The parser can only deal with char[].
hence, doing all of this data movement.
For now, I think this is okay. At least, I am not creating any objects and all the copying
is bulk operations which are relatively cheaper.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/951
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I71c3d8b8dfa5a98123725f139247d2b5ce10012e
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message