> On April 27, 2011, 4:16 a.m., Boroondas Gupte wrote: > > indra/llxml/llcontrol.cpp, lines 866-867 > > <http://codereview.secondlife.com/r/280/diff/1/?file=1516#file1516line866> > > > > 1) Any reason not to do the same for name? > > 2) Remove space between "const" and "&", so that it's easer to visually > > distinguish from binary operator & > > 3) LLSD::map_const_iterator seems to support operator ->, so we could > > write itr->second instead of (*itr).second .
1) Premature optimization is the root of all evil. In all seriousness though, I found that the recursive LLSD copying was causing a significant number of memory allocations. I didn't notice the name copying in the profiling, so I didn't look at it. Making this consistent is probably a good idea. 2) I think this is a bad idea, that would be far less readable imho. 3) I'd be fine with this change at some point, but I don't think it really matters. On April 27, 2011, 4:16 a.m., Brad Kittenbrink wrote: > > [*] Assuming the data referenced by control_map isn't changed by other > > threads meanwhile. But I guess that'd also be necessary for copying it > > correctly, isn't it? Correct, there are no other threads that can interfere with LLControlGroup::loadFromFile - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/280/#review655 ----------------------------------------------------------- On April 26, 2011, 5:36 p.m., Brad Kittenbrink wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/280/ > ----------------------------------------------------------- > > (Updated April 26, 2011, 5:36 p.m.) > > > Review request for Viewer. > > > Summary > ------- > > Unnecessary copying was slowing down debugging of STORM-1141. > > > This addresses bug VWR-25610. > http://jira.secondlife.com/browse/VWR-25610 > > > Diffs > ----- > > indra/llxml/llcontrol.cpp UNKNOWN > > Diff: http://codereview.secondlife.com/r/280/diff > > > Testing > ------- > > > Thanks, > > Brad > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
