tqchen edited a comment on pull request #7108: URL: https://github.com/apache/tvm/pull/7108#issuecomment-744864004
Thanks @tkonolige . First of all, I think it is very useful to be able to support additional meta data like names when the user can provid eit. On the other hand, attaching a String that defaults to a concrete value would mean additional overhead for PackedFunc creation, and may be prohibitive if we want to create PackedFunc on the fly(with a goal to gain std::function level speed). It would be good put more thoughts into this and think about how to improve the impl in general. As a first step, it certainly does not hurt to make the name Optional and default to nullptr, which reduces the string construction cost. We can alternatively think about the future path of bringing in PackedFunc as a sub-class of Object, and potentially sub-class PackedFunc. So the default one remains un-named, while there is a named subclass. Finally, it would be great to discuss the API itself, for example, set_name is a mutable API, while in most cases we would prefer things to be immutable. Given it is quite a core change, an RFC would be great. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
