[petsc-dev] Fwd: Re: MatDestroySubMatrices_SeqAIJ and reference counting
Hong
hzhang at mcs.anl.gov
Mon Jan 15 13:10:54 CST 2018
Stefano:
This patch
diff --git a/src/mat/impls/aij/seq/aij.c b/src/mat/impls/aij/seq/aij.c
index df16825..5c40de6 100644
--- a/src/mat/impls/aij/seq/aij.c
+++ b/src/mat/impls/aij/seq/aij.c
@@ -2542,11 +2542,14 @@ PetscErrorCode
MatDestroySubMatrices_SeqAIJ(PetscInt n,Mat *mat[])
c = (Mat_SeqAIJ*)C->data;
submatj = c->submatis1;
if (submatj) {
- ierr = submatj->destroy(C);CHKERRQ(ierr);
- ierr = MatDestroySubMatrix_Private(submatj);CHKERRQ(ierr);
- ierr = PetscLayoutDestroy(&C->rmap);CHKERRQ(ierr);
- ierr = PetscLayoutDestroy(&C->cmap);CHKERRQ(ierr);
- ierr = PetscHeaderDestroy(&C);CHKERRQ(ierr);
+ if (--((PetscObject)C)->refct > 0) {
+ } else {
+ ierr = submatj->destroy(C);CHKERRQ(ierr);
+ ierr = MatDestroySubMatrix_Private(submatj);CHKERRQ(ierr);
+ ierr = PetscLayoutDestroy(&C->rmap);CHKERRQ(ierr);
+ ierr = PetscLayoutDestroy(&C->cmap);CHKERRQ(ierr);
+ ierr = PetscHeaderDestroy(&C);CHKERRQ(ierr);
+ }
} else {
should fix the problem. Let me know your thought.
Hong
On Sun, Jan 14, 2018 at 9:09 PM, Hong <hzhang at mcs.anl.gov> wrote:
> Stefano:
> Sequential version works because it does not use common data structure.
> The old version of MatCreateSubMatrices() does not reuse internal data
> structures, thus inefficient.
> During its optimization, the internal data structures are introduced.
>
> I'll check it and see if I can provide these type of usage.
>
> Hong
>
>
>> On Jan 14, 2018, at 7:16 AM, Hong <hzhang at mcs.anl.gov> wrote:
>>
>> Stefano,
>> MatCreateSubMatrices() must be destroyed by MatDestroySubMatrices()
>> because all submatrices in the array share common internal data structure
>> for reuse.
>>
>> + ierr = PetscObjectReference((PetscObject)submat);CHKERRQ(ierr);
>>
>> Only submat = submatrices[0] adds reference count.
>>
>> ierr = MatDestroySubMatrices(1,&submatrices);CHKERRQ(ierr);
>> Now, the internal common data structure is destroyed.
>>
>> + ierr = MatDestroy(&submat);CHKERRQ(ierr);
>> I guess submat becomes an orphan, it cannot be destroyed using previous
>> ops->destroy routine.
>>
>> Shall we allow user to pick a single matrix out of submatrices array, and
>> use/destroy it as a regular single matrix?
>>
>>
>> I accept the fact that the array of submatrices obtained by
>> MatCreateSubMatrices shall be destroyed by MatDestroySubMatrices.
>> However, not being able to take reference on one of the submats is not
>> properly PETSc philosophy :-), as the Mats returned do not behave like
>> proper Mat objects
>> How difficult is to allow this? and what is the common data structure the
>> submats share?
>>
>> Hong
>>
>> On Sat, Jan 13, 2018 at 1:09 PM, Stefano Zampini <
>> stefano.zampini at gmail.com> wrote:
>>
>>> Cc'ing petsc-dev
>>> ---------- Messaggio inoltrato ----------
>>> Da: "Stefano Zampini" <stefano.zampini at gmail.com>
>>> Data: 12 Gen 2018 9:32 PM
>>> Oggetto: Re: MatDestroySubMatrices_SeqAIJ and reference counting
>>> A: "Hong" <hzhang at mcs.anl.gov>
>>> Cc:
>>>
>>>
>>> Sorry, I do not understand your question. Where comes matrix B?
>>>
>>>
>>> It’s not important where matrix B comes from. The error is reproducible
>>> by just increasing the reference count of any of the submats. See below the
>>> patch for ex4.c
>>>
>>> Can you give me an example of this error?
>>>
>>>
>>> here you have
>>>
>>> *diff --git a/src/mat/examples/tests/ex4.c
>>> b/src/mat/examples/tests/ex4.c*
>>> *index 0555a54d9d..1d1961e4fa 100644*
>>> *--- a/src/mat/examples/tests/ex4.c*
>>> *+++ b/src/mat/examples/tests/ex4.c*
>>> @@ -68,7 +68,9 @@ int main(int argc,char **argv)
>>> ierr = MatView(submat,sviewer);CHKERRQ(ierr);
>>> ierr = PetscViewerRestoreSubViewer(PETSC_VIEWER_STDOUT_WORLD,PETSC_
>>> COMM_SELF,&sviewer);CHKERRQ(ierr);
>>> ierr = PetscViewerFlush(PETSC_VIEWER_STDOUT_WORLD);CHKERRQ(ierr);
>>> + ierr = PetscObjectReference((PetscObject)submat);CHKERRQ(ierr);
>>> ierr = MatDestroySubMatrices(1,&submatrices);CHKERRQ(ierr);
>>> + ierr = MatDestroy(&submat);CHKERRQ(ierr);
>>>
>>> /* Form submatrix with rows 2-4 and all columns */
>>> ierr = ISDestroy(&icol);CHKERRQ(ierr);
>>>
>>> Hong
>>>
>>> Hong,
>>>>
>>>> can you explain what’s the rationale behind calling explicitly the
>>>> Layout and header destroy for the submit case in the loop here
>>>> https://bitbucket.org/petsc/petsc/src/ac3af1b492556bac9
>>>> 856a6aee1c73992bd0b1779/src/mat/impls/aij/seq/aij.c?at=maste
>>>> r&fileviewer=file-view-default#aij.c-2531
>>>>
>>>> A code like this fails since you don’t take into account reference
>>>> counting on the submatrices.
>>>>
>>>> MatCreateSubMatrices(A,….&submats)
>>>> PetscObjectCompose(B,”_XXXX”,submats[0);
>>>> …..
>>>> MatDestroySubMatrices(..,submats);
>>>> MatDestroy(B); //Error, corrupt argument when trying to destroy
>>>> composed objects
>>>>
>>>> Here is a representative valgrind stack trace
>>>>
>>>> ==90133== Invalid read of size 4
>>>> ==90133== at 0x100368F63: PetscCheckPointer
>>>> (/Users/szampini/software/petsc/src/sys/error/checkptr.c:108)
>>>> ==90133== by 0x1001ACA9F: PetscObjectDereference
>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:564)
>>>> ==90133== by 0x10019AC07: PetscObjectListDestroy
>>>> (/Users/szampini/software/petsc/src/sys/objects/olist.c:154)
>>>> ==90133== by 0x1001A8176: PetscHeaderDestroy_Private
>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:115)
>>>> ==90133== by 0x100626CAD: MatDestroy (/Users/szampini/software/pets
>>>> c/src/mat/interface/matrix.c:1237)
>>>> ==90133== Address 0x106c4c320 is 1,632 bytes inside a block of size
>>>> 4,420 free'd
>>>> ==90133== at 0x10014C9F3: free (in /usr/local/Cellar/valgrind/3.1
>>>> 3.0/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
>>>> ==90133== by 0x100229B7A: PetscFreeAlign
>>>> (/Users/szampini/software/petsc/src/sys/memory/mal.c:88)
>>>> ==90133== by 0x10022D771: PetscTrFreeDefault
>>>> (/Users/szampini/software/petsc/src/sys/memory/mtr.c:309)
>>>> ==90133== by 0x100A7034D: MatDestroySubMatrices_SeqAIJ
>>>> (/Users/szampini/software/petsc/src/mat/impls/aij/seq/aij.c:2549)
>>>> ==90133== by 0x1006260BD: MatDestroySubMatrices
>>>> (/Users/szampini/software/petsc/src/mat/interface/matrix.c:6957)
>>>> ==90133== by 0x1006D82EC: _DestroyContainer
>>>> ==90133== by 0x1001AFA31: PetscContainerDestroy
>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:899)
>>>> ==90133== by 0x1001ACBE5: PetscObjectDereference
>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:566)
>>>> ==90133== by 0x10019AC07: PetscObjectListDestroy
>>>> (/Users/szampini/software/petsc/src/sys/objects/olist.c:154)
>>>> ==90133== by 0x1001A8176: PetscHeaderDestroy_Private
>>>> (/Users/szampini/software/petsc/src/sys/objects/inherit.c:115)
>>>> ==90133== by 0x100626CAD: MatDestroy (/Users/szampini/software/pets
>>>> c/src/mat/interface/matrix.c:1237)
>>>> ==90133== Block was alloc'd at
>>>> ==90133== at 0x10014C616: malloc (in /usr/local/Cellar/valgrind/3.1
>>>> 3.0/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
>>>> ==90133== by 0x1002299AC: PetscMallocAlign
>>>> (/Users/szampini/software/petsc/src/sys/memory/mal.c:48)
>>>> ==90133== by 0x10022CA73: PetscTrMallocDefault
>>>> (/Users/szampini/software/petsc/src/sys/memory/mtr.c:183)
>>>> ==90133== by 0x10022B328: PetscMallocA (/Users/szampini/software/pets
>>>> c/src/sys/memory/mal.c:396)
>>>> ==90133== by 0x100C12E05: MatCreate (/Users/szampini/software/pets
>>>> c/src/mat/utils/gcreate.c:89)
>>>> ==90133== by 0x100B839F6: MatCreateSubMatrices_MPIAIJ_Local
>>>> (/Users/szampini/software/petsc/src/mat/impls/aij/mpi/mpiov.c:2554)
>>>> ==90133== by 0x100B7DEE7: MatCreateSubMatrices_MPIAIJ
>>>> (/Users/szampini/software/petsc/src/mat/impls/aij/mpi/mpiov.c:2038)
>>>> ==90133== by 0x10066703A: MatCreateSubMatrices
>>>> (/Users/szampini/software/petsc/src/mat/interface/matrix.c:6806)
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20180115/823ee0f3/attachment.html>
More information about the petsc-dev
mailing list