Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/parallel fetch data #267

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Feature/parallel fetch data #267

wants to merge 13 commits into from

Conversation

tstelzer
Copy link
Contributor

@tstelzer tstelzer commented Jul 12, 2022 •
edited

This PR begins to parallelize the (presumably) computationally most expensive step: parsing source files. It does so by introducing a worker pool, which the main thread delegates to. Summarized changes:

  • Wrapped makeCacheItemFromFilePath with a thread worker pool via piscina.
  • Rewrote signature of makeCacheItemFromFilePath to make input/result serializable.
  • (style) Introduced prettier plugin to eslint. Arguably, this doesn't need to be part of this PR 🤔

This PR is considered WIP, with some things left to do:

Mandatory:

  • Serialize/deserialize user-supplied plugins. Currently, if plugins are configured, every thread will panic, as plugins can't be loaded.
  • Serialize/deserialize failures caused in the thread, specifically, anything that is nested in unknown errors (explicit errors should already be handled).
  • Optimize. We're only getting ("on my machine") a ~2x speedup compared to single-threaded. Might be related to my machine, but there's probably much more we can get here. Maybe related to the overhead of serialization? Playing around with thread-pool size or queue size might be profitable as well.

Optional:

  • Generalize worker pool code. Right now, the code is very specific to makeCacheItemFromFilePath.
  • Improve otel tracing in thread. Currently, it's starting an entirely separate trace.

@tstelzer
feat(@contentlayer): begin threaded building of source files
494ffc1 
EXPERIMENTAL: Consider this commit a chain of experimental changes,
which likely break the pacakge.
@tstelzer
docs: test parallelism changes with gatsby docs
71f286c
@tstelzer
docs: add TODO
aa764e9
@tstelzer
feat(@contentlayer): serialize DocumentTypeMapState
a0bff26
@tstelzer
build: add eslint prettier plugin
266d267
@tstelzer
feat(@contentlayer): make documentTypeMap explicit in makeCacheItemFr…
d814ae1 
…omFilePath
@tstelzer
feat: compute makeCacheItemFromFilePath via worker pool
a53243e
@tstelzer
feat: de-serializing errors
6d8d3a2
@tstelzer
fix: node worker (supposedly) OOM
d8784a6 
This fixes an issue where the piscina worker pool crashes with
`FATAL ERROR: v8::FromJust Maybe value is Nothing`

Supposedly, this is the worker running out of memory? No idea. As a fix,
we explicitly pick boundaries for queue and thread size in piscina.
@tstelzer
style: improve
b4dcb70
@tstelzer
feat: replace dummy with live tracing in worker
fcb0103
@tstelzer
build: unserialized plugin throwing
fbd13b3
@tstelzer
docs: update
5c827a3
@changeset-bot
Copy link

changeset-bot bot commented Jul 12, 2022

⚠️ No Changeset found

Latest commit: 5c827a3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@schickling schickling added the help wanted Extra attention is needed label Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
help wanted Extra attention is needed meta: never-stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@tstelzer @schickling