Cached Exceptions #101
Cached Exceptions #101
Conversation
| using namespace std; | ||
|
|
||
| auto& context = env().getContext(); | ||
| FunctionType* read_block_type = FunctionType::get(env().i64Type(), {env().i8ptrType(), |
There was a problem hiding this comment.
let's maybe create a helper function next to where the func tyoe read_block_f is declared (CodegenHelper?) to create the type of the read_block_function.
There was a problem hiding this comment.
and some test that length of argnames matches the functype.
| for(auto obj : _py_objects) | ||
| Py_XINCREF(obj); | ||
| python::unlockGIL(); | ||
| _normalPartitions = cop->cachedNormalPartitions(); |
There was a problem hiding this comment.
hm maybe we should add a struct which contains all of these results... -> would make the code more readable.
There was a problem hiding this comment.
i.e., normalPartitions + generalPartitions + fallbackPartitions + partitionGroups. Maybe DatasetPartitions? or PartitionCollection? open for good name suggestions here...
There was a problem hiding this comment.
that struct/class should also contain counts etc.
| cachedPartitionsMemory += p->bytesWritten(); | ||
| totalCachedPartitionsMemory += p->size(); | ||
| pos++; | ||
| size_t normalBytesWritten = 0; |
There was a problem hiding this comment.
this here looks again quite repetitive, maybe we can structure it a bit more?
| // create new partition on driver | ||
| auto driver = _context->getDriver(); | ||
|
|
||
| std::vector<std::tuple<size_t, PyObject*>> fallbackRows; |
There was a problem hiding this comment.
some of this code here seems quite repetitive. Maybe restructure in functions/structs?
LeonhardFS
left a comment
There was a problem hiding this comment.
a couple minor things, great work here!
| uint8_t **fallbackPartitions = new uint8_t*[_fallbackPartitions.size()]; | ||
| for (int i = 0; i < _fallbackPartitions.size(); ++i) | ||
| fallbackPartitions[i] = _fallbackPartitions[i]->lockWriteRaw(); | ||
| int64_t numFallbackPartitions = _fallbackPartitions.size(); |
There was a problem hiding this comment.
size_t ? maybe also init then the vector of pointers with that var...
|
lgtm once CI passes |
No description provided.