You are right, provided you are sure w is off the stack. If you can be
sure of that, it's better to derec the child as you add it.
I also answered my question about the use of jt->tbase.
Henry
On 5/23/2016 1:56 PM, Marshall Lochbaum wrote:
I don't see why tpopnotw would be any better than just leaving w off the
stack (or, perhaps, why leaving w off the stack is flawed). The only
reason these functions break our current system is that they add a
recursive object as a child of a non-recursive one. So calling derec on
the children solves the problem, and any other changes are for
performance. And tpopexcept is clearly going to be slower than just
tpop. So what's the benefit?
Marshall
On Sun, May 22, 2016 at 12:17:19PM -0400, Henry Rich wrote:
no, tpopnotw would not leave the w dangling - it would move it (see
my latest). It would work like this (you will have to reflect any
changes you made to tpop/tpush):
tpopexcept(old,w){
I wfound = 0; // set if there was a free of w
while(old<jt->tbase+jt->ttop){ // till we have freed back to 'old'
//?? question: why is 'old <' used rather than 'old !='? Aren't the
// blocks of the stack in accidental order, so that this test
would fail
// if the stack spans a block boundary, and the second block is at a
// lower address than the first?
if(1<jt->ttop) { // if the stack item is a pointer to a free block
A blocktofree = jt->tstack[--jt->ttop]; // fetch address to free
if(blocktofree==w)++wfound;else fr(blocktofree); // free it,
unless it's w
elsetf(); // stack item is a stack-block chain pointer - free
the stack block
}
DO(wfound, tpush(w)); // restore any occurrences of w that were
suppressed
R old; // return free-to pointer - it may not be the top, if w was
restored
}
I agree that if it's w being changed, nothing special is needed, but
this would be a good idea for the other places.
What testcase would you like me to look at first?
Henry
On 5/22/2016 11:52 AM, Marshall Lochbaum wrote:
Replying to both of your comments here.
Almost all uses of this pattern actually replace w rather than append to
it. I haven't tested, but I think it's fine in all of these cases to use
gc or gc3. The exceptions which do modify w are in cp.c and vgauss.c,
although it's possible I missed some.
tpopnotw isn't possible--the stack is stored in an array, so ignoring w
would just leave it dangling beyond the end of the stack. Your first
suggestion still works, except that derec, rather than ra, makes objects
non-recursive.
I've pushed my changes so far to the unbox branch refcount:
https://github.com/iocane/unbox/commits/refcount
Failing tests are
- Because 7!:0 is no longer accurate
core/g202.ijs
core/g202b.ijs
core/g7x.ijs
core/glocale.ijs
- Problem with in-place sparse amends
sparse/gsp530i.ijs
sparse/gsp530l.ijs
sparse/gsp530n.ijs
- Miscellaneous
core/g13x.ijs
core/g5x2.ijs
core/gq.ijs
Presumably the last two groups are use-after-frees. Standard tools don't
work to identify these because of J's pool allocation, and making J use
mallocs instead of pool allocations causes bugs for reasons that are
probably even harder to track down. I have tried to configure Valgrind
to recognize pool allocations (using the requests described at
http://www.network-theory.co.uk/docs/valgrind/valgrind_57.html), but it
seems to only recognize that certain reads/writes are invalid, not the
free that caused them to be invalid. Useless.
Marshall
On Sat, May 21, 2016 at 04:29:03PM -0400, Henry Rich wrote:
store the current stack top in variable old.
for(...) {
add something to boxed array w.
call gc(w,old), putting w in non-recursive form.
}
What I was suggesting is:
store the current stack top in variable old.
ra(w);
for(...) {
add something to boxed array w.
ra(something); // putting something in non-recursive form.
tpop(old);
}
tpush(w);
Now I suggest
store the current stack top in variable old.
for(...) {
add something to boxed array w.
ra(something); // putting something in non-recursive form?
tpopnotw(old,w);
}
Where tpopnotw is like tpop but simply ignores stack entries that match w.
If this idea is sound, I think you might as well do it, because
* you will have to inspect each place gc() is used anyway
* assuming that the current code is there because the free was needed,
this preserves that behavior. It will be hard to be confident that
this is not needed (you'd have to deploy and wait for user complaints -
I wouldn't want to take that chance)
* No usecount will be modified or block freed that doesn't need to be
* it is necessary to increment the usecount of (something)
Henry
On 5/20/2016 10:06 AM, Marshall Lochbaum wrote:
A number of spots in the J source follow this pattern, or similar:
store the current stack top in variable old.
for(...) {
add something to boxed array w.
call gc(w,old), putting w in non-recursive form.
}
(A particular example is cp.c, lines 91--97, which performs (v^:l x)
where l is an integer list).
----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm
----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm
----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm
----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm
----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm
