Skip to content

Conversation

@benjaminweb
Copy link

(let me know if you'd prefer to load the html on test run instead of including it as file within the repo)

measureMemory :: T.Text -> IO ()
measureMemory t = do
m <- measure (nf (scrapeStringLike t) (texts "h2")) 1
let pma = (show . measPeakMbAllocated . fst) m
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like this was introduced in criterion 1.6.

Could you try adding that to extra-deps in stack.yaml? If that doesn't work let's just submit as is and I'll fiddle with the github actions to exclude benchmarks on older resolvers.

Copy link
Author

Choose a reason for hiding this comment

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

measPeakMbAllocated as well as measure come with criterion-measurement: https://hackage.haskell.org/package/criterion-measurement-0.2.3.0/docs/Criterion-Measurement-Types.html
I've already added that under scalpel-core.cabal bench section

Did I miss something? Let me know if…

Copy link
Owner

Choose a reason for hiding this comment

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

The CI is failing on older stack resolvers that have a version of criterion-measurement before measPeakMbAllocated was introduced.

I think you can fix those by adding an extra-dep with a more recent version, e.g. 0.2.3.0, similar to here (I don't think the sha is actually necessary).

let nested10000 = makeNested 10000
-- permalink: https://en.wikipedia.org/w/index.php?title=New_York_City&oldid=1292263955
wikipediaArticle <- T.readFile "benchmarks/wikipedia-new-york-city.html"
-- statsEnabled <- getGCStatsEnabled
Copy link
Owner

Choose a reason for hiding this comment

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

Left over? Should this be removed?

Copy link
Author

Choose a reason for hiding this comment

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

Removed leftover; I left the source location of the file on purpose.

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.

2 participants