ok. I agree. Thank you. Sriraj > On Jan 5, 2017, at 1:55 AM, Greg Titus <[email protected]> wrote: > > >> On Jan 4, 2017, at 12:06 PM, sri raj paul <[email protected]> wrote: >> >>> By the way, I want the caller of chpl_task_idTostring() to pass a buffer >>> (and length) to put the string in >> Any reason to pass the length? Api’s like sprintf do similar things with >> only the buffer. The caller of chpl_task_idTostring() should ensure the >> buffer is big enough. > > Just safety. Even with CHPL_TASK_ID_STRING_MAX_LEN defined (and with an > accurate value) I don’t think it’s reasonable for us to require the caller to > provide a buffer that big. That value is just the one we guarantee is the > most we’ll need. If the caller is building up a text message piece by piece > by appending in a fixed buffer, which is a somewhat common thing to do, when > chpl_task_idToString() is called there may not be CHPL_TASK_ID_STRING_MAX_LEN > bytes left in the buffer. (And it may be hard for that caller to guarantee > that there are, if there are other variably-sized pieces in the message.) As > long as the passed length accurately reflects the space available, > chpl_task_idToString() can use snprintf() to guarantee it doesn’t overflow > the buffer. But if it doesn’t know the length then it has to use sprintf() > and if for any reason the caller’s buffer isn’t large enough, it’ll overflow. > > greg > > >> >> Thank you >> Sriraj >> >>> On Jan 4, 2017, at 11:24 PM, Greg Titus <[email protected]> wrote: >>> >>> Pulling this up to the top … >>> >>>> This approach sounds fine to me too. I'd probably call the methods >>>> 'chpl_task_idToString' and 'chpl_task_idEquals' but chpl_task_sameIDs >>>> is OK if Greg really wants it to be that way :) >>> >>> I considered chpl_task_idEquals and ended up not liking it because we’ll be >>> passing two IDs in and it seemed more natural to call it >>> chpl_task_idsEqual, which I also wasn’t that fond of. Any of the 3 will >>> work, though — coder’s choice. :-) >>> >>> >>>> Sriraj, can we rely on you to create a pull request making these >>>> changes? (at least if nobody else chimes in…) >>> >>> I would envision a total of either 3 or 4 PRs: >>> • add chpl_task_idEquals() (or whatever) and chpl_task_idToString() to >>> the existing public tasking implementations >>> • (optional) the massiveThreads folks add the above to their tasking >>> implementation (but could also be included in the first PR) >>> • add the above to the Cray-private muxed tasking implementation (has >>> to be done by the Chapel core team) >>> • change to using these two functions in qio, chplvis, and anything >>> else that needs them >>> >>> The important thing is to make sure that last one doesn’t happen until all >>> the ones before have, so that we don’t end up not being able to build the >>> runtime in some configurations. >>> >>> By the way, I want the caller of chpl_task_idTostring() to pass a buffer >>> (and length) to put the string in, instead of allocating it dynamically. I >>> expect the string will rarely (never?) need to outlive the caller’s >>> activation record, so let’s avoid the allocation overhead. This probably >>> means we also need the implementations to define something like a >>> CHPL_TASK_ID_STRING_MAX_LEN preprocessor value that callers can use to >>> ensure their buffer is big enough for any legal task ID representation. >>> >>> greg >>> >>> >>>> On Jan 4, 2017, at 10:27 AM, Michael Ferguson <[email protected]> wrote: >>>> >>>> Hi - >>>> >>>> On 1/4/17, 11:57 AM, "Greg Titus" <[email protected]> wrote: >>>> >>>>> Actually, given Sriraj’s latest response I’ve come around to the other >>>>> point of view. >>>>> >>>>> Let’s separate the issues of the internal and the external task ID >>>>> representation. Even if the internal representation is a plain 64-bit >>>>> int defined independently from the implementations, defining a >>>>> chpl_task_sameIDs() to compare task IDs is a good idea so that the rest >>>>> of the code isn’t dependent on that internal type. That’s just good >>>>> interface engineering practice. >>>>> >>>>> Then, no matter what the internal representation is, only the >>>>> implementations can provide a reasonable chpl_task_idToString() >>>>> conversion function to produce the external representation, because only >>>>> the implementations can decode or unpack encoded internal IDs. For >>>>> example, if the encoded form has 16 high-order bits of node ID and 48 >>>>> low-order bits of task ID within node, if some ID has node==1 and >>>>> task_within_node==2 clearly it’s better to print “1:2” rather than the >>>>> raw value “281474976710658" or “0x100000002” an implementation >>>>> independent convertor would produce. >>>>> >>>>> Given that we need chpl_task_sameIDs() and chpl_task_idToString() anyway, >>>>> I believe we should continue to have the implementations define the task >>>>> ID type, so they can use whatever is convenient for them, and then >>>>> require them to provide those two functions (and adjust to using those in >>>>> qio and chplvis and elsewhere as needed). >>>> >>>> This approach sounds fine to me too. I'd probably call the methods >>>> 'chpl_task_idToString' and 'chpl_task_idEquals' but chpl_task_sameIDs >>>> is OK if Greg really wants it to be that way :) >>>> >>>> Sriraj, can we rely on you to create a pull request making these >>>> >>>> changes? (at least if nobody else chimes in...) >>>> >>>> -michael >>> >> >
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ Chapel-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/chapel-developers
