Schema performance improvements #632
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## mli-feature #632 +/- ##
==============================================
Coverage ? 76.61%
==============================================
Files ? 100
Lines ? 6905
Branches ? 0
==============================================
Hits ? 5290
Misses ? 1615
Partials ? 0
|
| msg_tensor = MessageHandler.build_tensor( | ||
| tensor, | ||
|
|
||
| # TODO isn't this what output descriptors are for? |
There was a problem hiding this comment.
Can we resolve these two TODO comments? Do we need to make tickets or can they be deleted?
There was a problem hiding this comment.
That's actually a note so I remembered to ask this question for the group! We don't use OutputDescriptors anywhere. I think the hardcoded information here can come from the OutputDescriptors so we know how the tensor needs to be reconstructed. I'll make a ticket for further discussion and remove these TODOs.
|
|
||
| interm = time.perf_counter() # timing | ||
| request = deserialize_message( | ||
| request_bytes, self._comm_channel_type, self._device | ||
| ) | ||
|
|
||
| if request.input_meta and tensor_list: |
There was a problem hiding this comment.
Maybe The logic from 248 to 264 (and the deserialization_message() content) would be better encapsulated in a unpack_request. I think _on_iteration should have minimal manipulation of the request based on the serialiation and communication specifics.
There was a problem hiding this comment.
(Does this make it more difficult to do perf timing though?)
There was a problem hiding this comment.
I completely agree, but maybe we wait to refactor _on_iteration until we're solid with performance timing? It might make it more difficult.
Performance improvements needed to be made in order to reduce the amount of copies we were making and de/serialization time. Instead of building a
Tensorand then adding it to aRequest, theRequestholdsTensorDescriptorsand the actual tensor data is sent after the request through theFLInterface.I was able to delete a lot of
TensorFlowandTorchtests that were separated out now that thebuild_tensorhas been updated tobuild_tensor_descriptor.