Skip to content

avoid struct move on Solaris#4845

Merged
mattdowle merged 1 commit intomasterfrom
solaris_openmp
Dec 15, 2020
Merged

avoid struct move on Solaris#4845
mattdowle merged 1 commit intomasterfrom
solaris_openmp

Conversation

@mattdowle
Copy link
Copy Markdown
Member

Closes #4099
Allocates the set of structs before the parallel region to avoid the move on Solaris
Adds the extra trace point mentioned in #4099, in case this theory is wrong
Will remove the word "should" in the news item after release, if it is successful.

@mattdowle mattdowle added this to the 1.13.5 milestone Dec 15, 2020
Comment thread src/fwrite.c
if (verbose) {DTPRINT(_("z_stream for data (%d): "), 2); print_z_stream(&mystream);}
int ret = compressbuff(&mystream, myzBuff, &myzbuffUsed, myBuff, (size_t)(ch-myBuff));
if (verbose) {DTPRINT(_("z_stream for data (%d): "), 3); print_z_stream(&mystream);}
if (verbose) {DTPRINT(_("z_stream for data (%d): "), 3); print_z_stream(mystream);}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

question: supposing this works, would you then aim to remove all the extra verbose printing for tracing in the next release? (less maintenance overhead / tidier code vs. ease of setting up the in-depth tracing should a similar error recur later on)

curious your thinking

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes will just remove the extra tracing. can always get it back from the history should we need it again

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 15, 2020

Codecov Report

Merging #4845 (83265ed) into master (d405d72) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4845   +/-   ##
=======================================
  Coverage   99.47%   99.47%           
=======================================
  Files          73       73           
  Lines       14589    14592    +3     
=======================================
+ Hits        14512    14515    +3     
  Misses         77       77           
Impacted Files Coverage Δ
src/fwrite.c 97.84% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d405d72...83265ed. Read the comment docs.

@mattdowle mattdowle merged commit 679c7ee into master Dec 15, 2020
@mattdowle mattdowle deleted the solaris_openmp branch December 15, 2020 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fwrite gzip on Solaris error

2 participants