Hi - On 1/4/17, 2:06 PM, "sri raj paul" <[email protected]> wrote:
> > > >Looks good. Buffer and CHPL_TASK_ID_STRING_MAX_LEN is a good idea. > > > >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. It's important to verify the length is big enough before writing to it (or else you write a stack overflow). sprintf in particular is pretty heavily deprecated / warned against in favor of snprintf. I think it's important to pass the length, even if it's always CHPL_TASK_ID_STRING_MAX_LEN in all cases that use it at the moment. -michael > > >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
