Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Enables NIO-based Concurrent Execution - #164

Closed
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution
Closed

Enables NIO-based Concurrent Execution#164
NeedleInAJayStack wants to merge 3 commits into
GraphQLSwift:mainfrom
NeedleInAJayStack:feat/concurrent-execution

Conversation

@NeedleInAJayStack

Copy link
Copy Markdown
Member

The existing concurrent executor was a mix of DispatchQueue.async calls and NIO futures, and was only used on an opt-in basis.

This makes the following changes:

  1. Adds a fully NIO-based concurrent executor
  2. Makes it the default executor used on query and subscription operations
  3. Deprecates APIs that allow specifying particular executors, as well as the DispatchQueue-based executor.

This should better parallelize GraphQL queries by default, resulting in improved performance.

Changes to default values are typically considered major, however this seems to be an exception. The GraphQL spec states the resolvers of queries must be side-effect free and idempotent, the execution order must not affect the result, which means that this can be considered a minor based on the resolver idempotency agreement.

The reasoning for the deprecation is that the JavaScript reference implementation does not expose executor implementations to the caller, instead always executing query and subscription concurrently and mutation serially. Furthermore, Graphiti does not allow customization of executors, implying that it is a rarely used feature.

@NeedleInAJayStackNeedleInAJayStack self-assigned this Jan 26, 2025
@NeedleInAJayStackNeedleInAJayStack changed the title Enables nio-based Concurrent ExecutionEnables NIO-based Concurrent ExecutionJan 26, 2025
@paulofaria

Copy link
Copy Markdown
Member

Is this good for review?

@NeedleInAJayStack
NeedleInAJayStackforce-pushed the feat/concurrent-execution branch from 24b5606 to 668c217CompareJune 21, 2025 06:12
@adam-fowler

Copy link
Copy Markdown
Contributor

Any reason you aren't moving to Swift concurrency?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

Any reason you aren't moving to Swift concurrency?

No, no reason in particular. This was opened back in January, and I was hoping that it would be a pretty small change to align with the intuition of a Concurrent executor.

You make a great point though - if we're going to make a breaking change, I think it makes sense for us to move completely onto Swift Concurrency and drop NIO support. I've started an MR for that here: #166 I'd love your feedback!!

@paulofaria

Copy link
Copy Markdown
Member

@NeedleInAJayStack can I close this PR?

@NeedleInAJayStack

Copy link
Copy Markdown
MemberAuthor

@paulofaria Yeah, let's close it.

Closing in favor of #166

Sign up for freeto 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.

3 participants

@NeedleInAJayStack@paulofaria@adam-fowler