Skip to content

Commit

Permalink
feat: Remove some OpenMP critical sections
Browse files Browse the repository at this point in the history
In one case this is no longer needed (now that the relevant code is object-oriented). In the other a file lock is a better solution.
  • Loading branch information
abensonca committed Nov 28, 2023
1 parent f411bee commit d00468f
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 6 deletions.
2 changes: 0 additions & 2 deletions source/merger_trees.operators.augment.F90
Original file line number Diff line number Diff line change
Expand Up @@ -427,14 +427,12 @@ subroutine augmentOperatePreEvolution(self,tree)
end do
! Accumulate the histogram of rescalings.
if (nodeBranches) then
!$omp critical (Augment_Statistics)
self %retryHistogram(min(rescaleCount,ubound(self%retryHistogram)))= &
& +self%retryHistogram(min(rescaleCount,ubound(self%retryHistogram))) &
& +1
self %trialCount (min(rescaleCount,ubound(self%retryHistogram)))= &
& +self%trialCount (min(rescaleCount,ubound(self%retryHistogram))) &
& +max(1,1+retryCount)
!$omp end critical (Augment_Statistics)
end if
! Clean up the best tree if one exists.
if (associated(treeBest%nodeBase)) then
Expand Down
2 changes: 0 additions & 2 deletions source/merger_trees.operators.export.F90
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,7 @@ subroutine exportOperatePreEvolution(self,tree)
end if
if (self%snapshotsRequired) call mergerTrees%setProperty(propertyTypeSnapshot,nodeSnapshot)
! Write the tree to file.
!$omp critical (Merger_Tree_Write)
call mergerTrees%export(char(self%outputFileName),self%exportFormat,hdfChunkSize,hdfCompressionLevel,append=.true.)
!$omp end critical (Merger_Tree_Write)
! Deallocate arrays.
deallocate(treeIndex )
deallocate(treeWeight )
Expand Down
8 changes: 6 additions & 2 deletions source/objects.merger_tree_data.F90
Original file line number Diff line number Diff line change
Expand Up @@ -1503,22 +1503,25 @@ subroutine Merger_Tree_Data_Structure_Export(mergerTrees,outputFileName,outputFo
!!{
Output a set of merger trees to an HDF5 file.
!!}
use :: Error, only : Error_Report
use :: HDF5 , only : hsize_t
use :: Error , only : Error_Report
use :: HDF5 , only : hsize_t
use :: File_Utilities, only : lockDescriptor, File_Lock, File_Unlock
implicit none
integer (kind=hsize_t ), intent(in ) :: hdfChunkSize
integer , intent(in ) :: hdfCompressionLevel
type (enumerationMergerTreeFormatType), intent(in ) :: outputFormat
class (mergerTreeData ), intent(inout) :: mergerTrees
character(len=* ), intent(in ) :: outputFileName
logical , intent(in ), optional :: append
type (lockDescriptor ) :: fileLock

! Validate the merger tree.
call Merger_Tree_Data_Validate_Trees (mergerTrees)

! If we have most-bound particle indices and particle data has been read, construct arrays giving position of particle data for each node.
call Merger_Tree_Data_Construct_Particle_Indices(mergerTrees)

call File_Lock(outputFileName,fileLock,lockIsShared=.false.)
select case (outputFormat%ID)
case (mergerTreeFormatGalacticus%ID)
call Merger_Tree_Data_Structure_Export_Galacticus(mergerTrees,outputFileName,hdfChunkSize,hdfCompressionLevel,append)
Expand All @@ -1527,6 +1530,7 @@ subroutine Merger_Tree_Data_Structure_Export(mergerTrees,outputFileName,outputFo
case default
call Error_Report('output format is not recognized'//{introspection:location})
end select
call File_Unlock(fileLock)
return
end subroutine Merger_Tree_Data_Structure_Export

Expand Down

1 comment on commit d00468f

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Milky Way model benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.10.

Benchmark suite Current: d00468f Previous: f411bee Ratio
Milky Way model - Likelihood - localGroupMassSizeRelation 5.65549160927224 -logℒ 4.37163159732478 -logℒ 1.29

This comment was automatically generated by workflow using github-action-benchmark.

CC: @abensonca

Please sign in to comment.