Opentelemetry traces and heapsize memory analysis #922

pepeiborra · 2020-11-25T10:53:56Z

This PR builds on the work of @mpardalos and supersedes #826

A new facility to log open telemetry events and spans to the GHC event log is added. The following are logged:

LSP events (all of them)
Shake action spans

Additionally, when the flag --ot-memory-profiling is given, the following are logged periodically:

Count of items in the ShakeExtras "state" (Metric is called "values map size_bytes")
Aggregate size of items per key, with sharing, so that shared costs are not reported twice.

For the non LSP driver, the flag --ot-memory-profiling produces a textual version of the above, instead of an eventlog.

There is documentation on how to visualise the eventlogs using Tracy. The Nix script takes care of installing all the dependencies

TODOs:

Merge my heapsize changes in Avoid segfaults and allow sharing mpardalos/Heapsize#1, release a new version to Hackage and remove the git submodule

Future TODOs (not for this PR):

Use spans to link LSP events and Shake actions, as well as Shake dependencies
--ot-memory-profiling should be always enabled and log only on SIGHUP(or some other signal) instead of periodically
Output mechanism for traces should be configurable, so that we can log to stdout, the cloud, etc.

Instead of doing it in `HoverDefinition`, do it in with{Response,Notification,...}. These wrap all handlers, so this should cover everything. It also means that the span covers the entire processing time for the request, where before we missed the setup happening in the with* functions.

Run GC regularly with --ot-profiling

I renamed the fork to distringuish from the original. It is still being pulled from git using stack. This will be addressed once I can push the fork to hackage.

to avoid circular module dependencies

No more segfaults

When using ghcide as a library, it may be desirable to host the hie.yaml file in a location other than the project root, or even avoid the file system altogether

* Favor `lookupPathToId` over `pathToId` * Fix `typecheckParentsAction` * Fix `needsCompilationRule`

* Use the real client capabilities on completions * Return completion snippets only when supported by the client Restored from haskell#900

jneira · 2020-11-30T10:08:01Z

@pepeiborra will try to take a look asap, but the error does not look good, we could disable temporaly the windows build if you want to go forward and activate it afterwards.
In hls there is no stack build for windows so will no suffer it.

jneira · 2020-11-30T13:44:13Z

stack-windows.yaml

+# > To load a package foo, GHCi can load its libHSfoo.a library directly, but it can also load a package in the form of a single HSfoo.o file that has been pre-linked. Loading the .o file is slightly quicker, but at the expense of having another copy of the compiled package. The rule of thumb is that if the modules of the package were compiled with -split-sections then building the HSfoo.o is worthwhile because it saves time when loading the package into GHCi. Without -split-sections, there is not much difference in load time between the .o and .a libraries, so it is better to save the disk space and only keep the .a around. In a GHC distribution we provide .o files for most packages except the GHC package itself.
+# > The HSfoo.o file is built by Cabal automatically; use --disable-library-for-ghci to disable it. To build one manually, the following GNU ld command can be used:
+
+# > ld -r --whole-archive -o HSfoo.o libHSfoo.a


OMG 🤦

Thanks for the detailed report of the root cause

Yeah, but the fix is not enough. There is another stack issue after it.

pepeiborra · 2020-11-30T19:03:44Z

@jneira @ndmitchell unless one of you can get to the bottom of the problem here, I will drop the Stack Windows build as we lack the maintainer power to keep it working.

jneira · 2020-11-30T22:07:25Z

The error seems to be of the same kind:

ghcide> ghc.exe:  | D:\a\1\s\.stack-work\dist\a3a5fe88\build\HSghcide-0.5.0-GfeKdUCOB448C8Yqgu22oa.o: unknown symbol `heapsizzezm0zi3zi0zmG6oW1FfbuDgJS3obQNcUGF_HeapSizze_runHeapsizze1_closure'

Reproduced in local

The Stack Windows build is problematic: haskell#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

pepeiborra · 2020-12-03T23:17:44Z

#934 opened to unblock this PR

The Stack Windows build is problematic: haskell#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

The Stack Windows build is problematic: #922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

The Stack Windows build is problematic: haskell/ghcide#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

* Move tracing functions to own module * Bump opentelemetry to 0.6.0 * Write Values map size to OpenTelemetry metric * Trace all requests and notifications Instead of doing it in `HoverDefinition`, do it in with{Response,Notification,...}. These wrap all handlers, so this should cover everything. It also means that the span covers the entire processing time for the request, where before we missed the setup happening in the with* functions. * Add flag for OpenTelemetry profiling Run GC regularly with --ot-profiling * Add flag to enable OT profiling in benchmark * Use heapsize instead of ghc-datasize I renamed the fork to distringuish from the original. It is still being pulled from git using stack. This will be addressed once I can push the fork to hackage. * Bump opentelemetry to 0.6.1 - fixes 8.6 build * Use heapsize from hackage * Address HLint messages * Record size of each key independently * Refactor `startTelemetry` function * Remove delay between measuring memory loops * Each key in values map gets own OT instrument * Measure values map length more rarely * Rename --ot-profiling to --ot-memory-profiling * Add docs for how to use the opentelemetry output * Add instructions to build release version of tracy * Clarify dependencies in opentelemetry instructions * Fix LSP traces * otTraced: delete unused * Extract types out of D.IDE.Core.Shake to avoid circular module dependencies * Extract startTelemetry out of D.IDE.Shake and upgrade to 0.2 No more segfaults * [nix] install opentelemetry * [nix] install tracy * Fix merge wibble * Measure recursive sizes with sharing * Sort keys for cost attribution * Remove debug traces * Allocate less, group keys, clean up hlints * Add -A4G to the flags used for --ot-memory-profiling * Modularize D.IDE.Core.Tracing I want to reuse this code more directly in the non lsp driver * Direct driver: report closure sizes when --ot-memory-profiling An eventlog memory analysis doesnt' seem so relevant since this mode is not interactive, but we could easily produce both if wanted to * Everything is reachable from GhcSessionIO, so compute it last I suspect the ShakeExtras record is reachable from GhcSessionIO * bound recursion and use logger * hlint suggestions * Fix 8.6 build * Format imports * Do the memory analysis with full sharing. GhcSessionIO last * Fail fast in the memory analysis * error handling * runHeapsize now takes initSize as an input argument * Trace Shake sessions * Reduced frequency for sampling values length * Drop the -fexternal-interpreter flag in the Windows stack build * Produce more benchmark artifacts * Fix stack descriptors to use heapsize-0.2 from Hackage * Bump to heapsize-0.3.0 * Record completions snippets (haskell/ghcide#900) * Add field for RecordSnippets to CachcedCompletion * Initial version of local record snippets * Supprt record snippet completion for non local declarations. * Better integration of local completions with current implementation * Clean up non-local completions. * Remove commented code. * Switch from String to Text * Remove ununsed definition * Treat only Records and leave other defintions as is. * Differentiate Records from Data constructors for external declaration * Update test to include snippet in local record completions expected list. * Update completionTest to also compare insertText. * Add test for record snippet completion for imported records. * Hlint fixes * Hlint fixes * Hlint suggestions. * Update type. * Consolidate imports * Unpack tuple with explicit names * Idiomatic changes * Remove unused variable * Better variable name * Hlint suggestions * Handle exhaustive pattern warning * Add _ to snippet field name suggestions * Remove type information passed around but not used * Update to list comprehension style * Eliminate intermediate function * HLint suggestions. * Idiomatic list comprehension Co-authored-by: Pepe Iborra <[email protected]> * [nix] use gitignore.nix (haskell/ghcide#920) * Ignore import list while producing completions (haskell/ghcide#919) * Drop any items in explicit import list * Test if imports not included in explicit list show up in completions * Update README.md (haskell/ghcide#924) * Custom cradle loading (haskell/ghcide#928) When using ghcide as a library, it may be desirable to host the hie.yaml file in a location other than the project root, or even avoid the file system altogether * Favor `lookupPathToId` over `pathToId` (haskell/ghcide#926) * Favor `lookupPathToId` over `pathToId` * Fix `typecheckParentsAction` * Fix `needsCompilationRule` * Return completion snippets only when client supports it (haskell/ghcide#929) * Use the real client capabilities on completions * Return completion snippets only when supported by the client Restored from haskell/ghcide#900 * Redundant import * Fix stack windows build Co-authored-by: Michalis Pardalos <[email protected]> Co-authored-by: Michalis Pardalos <[email protected]> Co-authored-by: Guru Devanla <[email protected]> Co-authored-by: Samuel Ainsworth <[email protected]>

The Stack Windows build is problematic: haskell/ghcide#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

* Move tracing functions to own module * Bump opentelemetry to 0.6.0 * Write Values map size to OpenTelemetry metric * Trace all requests and notifications Instead of doing it in `HoverDefinition`, do it in with{Response,Notification,...}. These wrap all handlers, so this should cover everything. It also means that the span covers the entire processing time for the request, where before we missed the setup happening in the with* functions. * Add flag for OpenTelemetry profiling Run GC regularly with --ot-profiling * Add flag to enable OT profiling in benchmark * Use heapsize instead of ghc-datasize I renamed the fork to distringuish from the original. It is still being pulled from git using stack. This will be addressed once I can push the fork to hackage. * Bump opentelemetry to 0.6.1 - fixes 8.6 build * Use heapsize from hackage * Address HLint messages * Record size of each key independently * Refactor `startTelemetry` function * Remove delay between measuring memory loops * Each key in values map gets own OT instrument * Measure values map length more rarely * Rename --ot-profiling to --ot-memory-profiling * Add docs for how to use the opentelemetry output * Add instructions to build release version of tracy * Clarify dependencies in opentelemetry instructions * Fix LSP traces * otTraced: delete unused * Extract types out of D.IDE.Core.Shake to avoid circular module dependencies * Extract startTelemetry out of D.IDE.Shake and upgrade to 0.2 No more segfaults * [nix] install opentelemetry * [nix] install tracy * Fix merge wibble * Measure recursive sizes with sharing * Sort keys for cost attribution * Remove debug traces * Allocate less, group keys, clean up hlints * Add -A4G to the flags used for --ot-memory-profiling * Modularize D.IDE.Core.Tracing I want to reuse this code more directly in the non lsp driver * Direct driver: report closure sizes when --ot-memory-profiling An eventlog memory analysis doesnt' seem so relevant since this mode is not interactive, but we could easily produce both if wanted to * Everything is reachable from GhcSessionIO, so compute it last I suspect the ShakeExtras record is reachable from GhcSessionIO * bound recursion and use logger * hlint suggestions * Fix 8.6 build * Format imports * Do the memory analysis with full sharing. GhcSessionIO last * Fail fast in the memory analysis * error handling * runHeapsize now takes initSize as an input argument * Trace Shake sessions * Reduced frequency for sampling values length * Drop the -fexternal-interpreter flag in the Windows stack build * Produce more benchmark artifacts * Fix stack descriptors to use heapsize-0.2 from Hackage * Bump to heapsize-0.3.0 * Record completions snippets (haskell/ghcide#900) * Add field for RecordSnippets to CachcedCompletion * Initial version of local record snippets * Supprt record snippet completion for non local declarations. * Better integration of local completions with current implementation * Clean up non-local completions. * Remove commented code. * Switch from String to Text * Remove ununsed definition * Treat only Records and leave other defintions as is. * Differentiate Records from Data constructors for external declaration * Update test to include snippet in local record completions expected list. * Update completionTest to also compare insertText. * Add test for record snippet completion for imported records. * Hlint fixes * Hlint fixes * Hlint suggestions. * Update type. * Consolidate imports * Unpack tuple with explicit names * Idiomatic changes * Remove unused variable * Better variable name * Hlint suggestions * Handle exhaustive pattern warning * Add _ to snippet field name suggestions * Remove type information passed around but not used * Update to list comprehension style * Eliminate intermediate function * HLint suggestions. * Idiomatic list comprehension Co-authored-by: Pepe Iborra <[email protected]> * [nix] use gitignore.nix (haskell/ghcide#920) * Ignore import list while producing completions (haskell/ghcide#919) * Drop any items in explicit import list * Test if imports not included in explicit list show up in completions * Update README.md (haskell/ghcide#924) * Custom cradle loading (haskell/ghcide#928) When using ghcide as a library, it may be desirable to host the hie.yaml file in a location other than the project root, or even avoid the file system altogether * Favor `lookupPathToId` over `pathToId` (haskell/ghcide#926) * Favor `lookupPathToId` over `pathToId` * Fix `typecheckParentsAction` * Fix `needsCompilationRule` * Return completion snippets only when client supports it (haskell/ghcide#929) * Use the real client capabilities on completions * Return completion snippets only when supported by the client Restored from haskell/ghcide#900 * Redundant import * Fix stack windows build Co-authored-by: Michalis Pardalos <[email protected]> Co-authored-by: Michalis Pardalos <[email protected]> Co-authored-by: Guru Devanla <[email protected]> Co-authored-by: Samuel Ainsworth <[email protected]>

The Stack Windows build is problematic: haskell/ghcide#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

* Move tracing functions to own module * Bump opentelemetry to 0.6.0 * Write Values map size to OpenTelemetry metric * Trace all requests and notifications Instead of doing it in `HoverDefinition`, do it in with{Response,Notification,...}. These wrap all handlers, so this should cover everything. It also means that the span covers the entire processing time for the request, where before we missed the setup happening in the with* functions. * Add flag for OpenTelemetry profiling Run GC regularly with --ot-profiling * Add flag to enable OT profiling in benchmark * Use heapsize instead of ghc-datasize I renamed the fork to distringuish from the original. It is still being pulled from git using stack. This will be addressed once I can push the fork to hackage. * Bump opentelemetry to 0.6.1 - fixes 8.6 build * Use heapsize from hackage * Address HLint messages * Record size of each key independently * Refactor `startTelemetry` function * Remove delay between measuring memory loops * Each key in values map gets own OT instrument * Measure values map length more rarely * Rename --ot-profiling to --ot-memory-profiling * Add docs for how to use the opentelemetry output * Add instructions to build release version of tracy * Clarify dependencies in opentelemetry instructions * Fix LSP traces * otTraced: delete unused * Extract types out of D.IDE.Core.Shake to avoid circular module dependencies * Extract startTelemetry out of D.IDE.Shake and upgrade to 0.2 No more segfaults * [nix] install opentelemetry * [nix] install tracy * Fix merge wibble * Measure recursive sizes with sharing * Sort keys for cost attribution * Remove debug traces * Allocate less, group keys, clean up hlints * Add -A4G to the flags used for --ot-memory-profiling * Modularize D.IDE.Core.Tracing I want to reuse this code more directly in the non lsp driver * Direct driver: report closure sizes when --ot-memory-profiling An eventlog memory analysis doesnt' seem so relevant since this mode is not interactive, but we could easily produce both if wanted to * Everything is reachable from GhcSessionIO, so compute it last I suspect the ShakeExtras record is reachable from GhcSessionIO * bound recursion and use logger * hlint suggestions * Fix 8.6 build * Format imports * Do the memory analysis with full sharing. GhcSessionIO last * Fail fast in the memory analysis * error handling * runHeapsize now takes initSize as an input argument * Trace Shake sessions * Reduced frequency for sampling values length * Drop the -fexternal-interpreter flag in the Windows stack build * Produce more benchmark artifacts * Fix stack descriptors to use heapsize-0.2 from Hackage * Bump to heapsize-0.3.0 * Record completions snippets (haskell/ghcide#900) * Add field for RecordSnippets to CachcedCompletion * Initial version of local record snippets * Supprt record snippet completion for non local declarations. * Better integration of local completions with current implementation * Clean up non-local completions. * Remove commented code. * Switch from String to Text * Remove ununsed definition * Treat only Records and leave other defintions as is. * Differentiate Records from Data constructors for external declaration * Update test to include snippet in local record completions expected list. * Update completionTest to also compare insertText. * Add test for record snippet completion for imported records. * Hlint fixes * Hlint fixes * Hlint suggestions. * Update type. * Consolidate imports * Unpack tuple with explicit names * Idiomatic changes * Remove unused variable * Better variable name * Hlint suggestions * Handle exhaustive pattern warning * Add _ to snippet field name suggestions * Remove type information passed around but not used * Update to list comprehension style * Eliminate intermediate function * HLint suggestions. * Idiomatic list comprehension Co-authored-by: Pepe Iborra <[email protected]> * [nix] use gitignore.nix (haskell/ghcide#920) * Ignore import list while producing completions (haskell/ghcide#919) * Drop any items in explicit import list * Test if imports not included in explicit list show up in completions * Update README.md (haskell/ghcide#924) * Custom cradle loading (haskell/ghcide#928) When using ghcide as a library, it may be desirable to host the hie.yaml file in a location other than the project root, or even avoid the file system altogether * Favor `lookupPathToId` over `pathToId` (haskell/ghcide#926) * Favor `lookupPathToId` over `pathToId` * Fix `typecheckParentsAction` * Fix `needsCompilationRule` * Return completion snippets only when client supports it (haskell/ghcide#929) * Use the real client capabilities on completions * Return completion snippets only when supported by the client Restored from haskell/ghcide#900 * Redundant import * Fix stack windows build Co-authored-by: Michalis Pardalos <[email protected]> Co-authored-by: Michalis Pardalos <[email protected]> Co-authored-by: Guru Devanla <[email protected]> Co-authored-by: Samuel Ainsworth <[email protected]>

mpardalos and others added 30 commits September 22, 2020 19:58

Move tracing functions to own module

e4d1761

Bump opentelemetry to 0.6.0

767f472

Write Values map size to OpenTelemetry metric

9423b0e

Add flag for OpenTelemetry profiling

4c6ab59

Run GC regularly with --ot-profiling

Add flag to enable OT profiling in benchmark

6940773

Use heapsize instead of ghc-datasize

1bbe02a

I renamed the fork to distringuish from the original. It is still being pulled from git using stack. This will be addressed once I can push the fork to hackage.

Bump opentelemetry to 0.6.1 - fixes 8.6 build

4d079e9

Use heapsize from hackage

53c9a40

Address HLint messages

a697ab8

Merge branch 'master' into opentelemetry-memory

4279bb6

Record size of each key independently

69ae2c7

Refactor startTelemetry function

b32d0d8

Remove delay between measuring memory loops

7ba4add

Each key in values map gets own OT instrument

f29acde

Merge remote-tracking branch 'upstream/master' into HEAD

087d814

Measure values map length more rarely

922b653

Rename --ot-profiling to --ot-memory-profiling

352609f

Add docs for how to use the opentelemetry output

1cb5837

Add instructions to build release version of tracy

1039864

Clarify dependencies in opentelemetry instructions

98bc3d9

Merge remote-tracking branch 'upstream/master' into opentelemetry

d8eb39f

Fix LSP traces

fa1a0fb

otTraced: delete unused

158a687

Extract types out of D.IDE.Core.Shake

68d4285

to avoid circular module dependencies

Extract startTelemetry out of D.IDE.Shake and upgrade to 0.2

42a29d0

No more segfaults

[nix] install opentelemetry

2ed6df5

[nix] install tracy

9f0967b

Merge branch 'opentelemetry-clean' into opentelemetry

b6adaa1

Fix merge wibble

1c03e84

samuela and others added 4 commits November 29, 2020 23:09

Update README.md (haskell#924)

ee1e08b

Custom cradle loading (haskell#928)

18c695d

When using ghcide as a library, it may be desirable to host the hie.yaml file in a location other than the project root, or even avoid the file system altogether

Favor lookupPathToId over pathToId (haskell#926)

57d2a9b

* Favor `lookupPathToId` over `pathToId` * Fix `typecheckParentsAction` * Fix `needsCompilationRule`

Return completion snippets only when client supports it (haskell#929)

b1745cd

* Use the real client capabilities on completions * Return completion snippets only when supported by the client Restored from haskell#900

pepeiborra force-pushed the opentelemetry branch from 9cdb83e to 0872d67 Compare November 29, 2020 23:10

Merge remote-tracking branch 'upstream/master' into opentelemetry

5503653

pepeiborra force-pushed the opentelemetry branch from 0872d67 to 771f7c8 Compare November 30, 2020 09:00

pepeiborra force-pushed the opentelemetry branch from db600f4 to ac6c5da Compare November 30, 2020 10:10

Redundant import

7c0a78a

pepeiborra force-pushed the opentelemetry branch from ac6c5da to 7c0a78a Compare November 30, 2020 11:21

Fix stack windows build

0c41056

jneira reviewed Nov 30, 2020

View reviewed changes

pepeiborra added a commit to pepeiborra/ghcide that referenced this pull request Dec 3, 2020

Drop stack Windows CI

d5f3153

The Stack Windows build is problematic: haskell#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

pepeiborra mentioned this pull request Dec 3, 2020

Drop stack Windows CI #934

Merged

pepeiborra added a commit to pepeiborra/ghcide that referenced this pull request Dec 5, 2020

Drop stack Windows CI

919d3bc

The Stack Windows build is problematic: haskell#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

pepeiborra added a commit that referenced this pull request Dec 5, 2020

Drop stack Windows CI (#934)

28f33cc

The Stack Windows build is problematic: #922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

Merge remote-tracking branch 'upstream/master' into opentelemetry

996bad9

pepeiborra force-pushed the opentelemetry branch from a531f9c to 996bad9 Compare December 5, 2020 15:27

pepeiborra merged commit e24a744 into haskell:master Dec 5, 2020

pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020

Drop stack Windows CI (haskell/ghcide#934)

38ac59c

The Stack Windows build is problematic: haskell/ghcide#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020

Drop stack Windows CI (haskell/ghcide#934)

88e295f

The Stack Windows build is problematic: haskell/ghcide#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

pepeiborra added a commit to pepeiborra/ide that referenced this pull request Dec 29, 2020

Drop stack Windows CI (haskell/ghcide#934)

332e2a1

The Stack Windows build is problematic: haskell/ghcide#922 Stack is already covered by the Azure CI Windows is already covered by the Github Actions CI

mpardalos mentioned this pull request Jan 19, 2021

Memory usage haskell/haskell-language-server#802

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opentelemetry traces and heapsize memory analysis #922

Opentelemetry traces and heapsize memory analysis #922

pepeiborra commented Nov 25, 2020 •

edited

Loading

jneira commented Nov 30, 2020

jneira Nov 30, 2020

pepeiborra Nov 30, 2020

pepeiborra commented Nov 30, 2020

jneira commented Nov 30, 2020 •

edited

Loading

pepeiborra commented Dec 3, 2020

Opentelemetry traces and heapsize memory analysis #922

Opentelemetry traces and heapsize memory analysis #922

Conversation

pepeiborra commented Nov 25, 2020 • edited Loading

jneira commented Nov 30, 2020

jneira Nov 30, 2020

Choose a reason for hiding this comment

pepeiborra Nov 30, 2020

Choose a reason for hiding this comment

pepeiborra commented Nov 30, 2020

jneira commented Nov 30, 2020 • edited Loading

pepeiborra commented Dec 3, 2020

pepeiborra commented Nov 25, 2020 •

edited

Loading

jneira commented Nov 30, 2020 •

edited

Loading