Table copy error has been fixed #104
Conversation
|
+1 |
|
Brian, I am not sure what you mean. For now this pull request is simply a bug-report. No attempt at fixing the bug has been made yet. Using pull requests to report bugs has been a long-time custom of mine, and of Alexander. |
…nto ks-table-copy-error
|
I found the error. It is due to the call to Array::CalcByteLen() in Array::Copy(). The problem is that CalcByteLen() is virtual and has a special implementation for string arrays, however, the recursion in Array::Copy() does not create StringArray objects, it always creates Array objects. The immediate fix is to avoid calling CalcByteLen(), but manually calculate the value in a way similar to what is done in Array::get_alloc_size_from_header(). An even better approach would be to change Array::Copy() so that it works directly on "refs" and thereby avoids instantiating array objects for string arrays. |
…Copy() has been replaced by Table::clone() whose semantics is better suited to our needs.
ghost
assigned
|
All unit tests pass, and valgrind reports no issues. |
…, Array::Copy() has been replaced by Table::clone() whose semantics is better suited to our needs.
| size_t size = get_byte_size_from_header(header); | ||
| // FIXME: Alexander, what is the reasoning behing this precise | ||
| // amount of extra capacity in the new array? | ||
| const size_t new_size = size + 64; |
There was a problem hiding this comment.
This is pretty much just an arbitrary number to give a bit of a buffer for future changes. It might actually be better to not give any buffer, and then just do the usual extension on inserts. That might reduce our memory footprint a bit in the (probably quite usual) cases, where it just get copied and no modified afterwards.
|
+1 |
Table copy error has been fixed
The table copy error has been fixed.
As part of the solution, Array::Copy() has been replaced by Table::clone() whose semantics is better suited to our needs.
The actual error was due to a call to Array::CalcByteLen() in Array::Copy().
Alexander, can you take a look at my fix? I believe you are the original author of Table::Copy().
\cc @astigsen