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

Rewriting Async API in ImageExport #321

Merged
merged 2 commits into from
Jul 11, 2022
Merged

Rewriting Async API in ImageExport #321

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b35cb37
#320, #306
WhiteBlackGoose Jul 9, 2022
bbeacb7
Tests added
WhiteBlackGoose Jul 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions src/Plotly.NET.ImageExport/AsyncHelper.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
module Plotly.NET.ImageExport.AsyncHelper

open System.Threading
open System.Threading.Tasks

(*

This is a workaround to avoid deadlocks

https://medium.com/rubrikkgroup/understanding-async-avoiding-deadlocks-e41f8f2c6f5d

TL;DR in many cases, for example, GUI apps, SynchronizationContext
is overriden to *post* the executing code on the initial (UI) thread. For example,
consider this code

public async Task OnClick1()
{
var chart = ...;
var base64 = ImageExport.toBase64PNGStringAsync()(chart).Result;
myButton.Text = base64;
}

Here we have an async method. Normally you should use await and not use .Result, but
assume for some reason the sync version is used. What happens under the hood is,

public async Task OnClick1()
{
var chart = ...;
var task = ImageExport.toBase64PNGStringAsync()(chart);
task.ContinueWith(() =>
UIThread.Schedule(() =>
myButton.Text = Result;
)
);
task.Wait();
}

(this is pseudo-code)

So basically, we set the task to wait until it finishes. However, part of it being
finished is to actually execute the code with button.Text = .... The waiting happens
on the UI thread, exactly on the same thread as where we're waiting for it to do
another job!

That's not the only place we potentially deadlock by using fake synchronous functions.
The reason why it happens, is because frameworks (or actually anyone) override
SynchronizationContext. In GUI and game development it's very useful to keep UI logic
on one thread. But our rendering does not ever callback to it, we're independent of
where the logic actually happens.

That's why what we do is we set the synchronization context to null, do the job, and
then restore it. It is a workaround, because it doesn't have to work everywhere and
independently. But it will work for most cases.

When will it also break? For example, if we decide to take in some callback as a para-
meter, which potentially accesses the UI thread (or whatever). In Unity, for instance,
you can only access Unity API from the main thread. So our fake synchronous function
will crash in the end, because due to the overriden (by us) sync context, the callback
will be executed in some random thread (as opposed to being posted back to the UI one).

However, our solution should work in most cases.

Credit to [@DaZombieKiller](https://github.com/DaZombieKiller) for helping.

*)

let runSync job input =
let current = SynchronizationContext.Current
SynchronizationContext.SetSynchronizationContext null
try
job input
finally
SynchronizationContext.SetSynchronizationContext current

let taskSync (task : Task<'a>) = task |> runSync (fun t -> t.Result)

let taskSyncUnit (task : Task) = task |> runSync (fun t -> t.Wait())
12 changes: 6 additions & 6 deletions src/Plotly.NET.ImageExport/ChartExtensions.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ module ChartExtensions =
fun (gChart: GenericChart) ->
gChart
|> Chart.toBase64JPGStringAsync (?EngineType = EngineType, ?Width = Width, ?Height = Height)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

/// <summary>
/// Returns an async function that saves a GenericChart as JPG image
Expand Down Expand Up @@ -97,7 +97,7 @@ module ChartExtensions =
fun (gChart: GenericChart) ->
gChart
|> Chart.saveJPGAsync (path, ?EngineType = EngineType, ?Width = Width, ?Height = Height)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

/// <summary>
/// Returns an async function that converts a GenericChart to a base64 encoded PNG string
Expand Down Expand Up @@ -134,7 +134,7 @@ module ChartExtensions =
fun (gChart: GenericChart) ->
gChart
|> Chart.toBase64PNGStringAsync (?EngineType = EngineType, ?Width = Width, ?Height = Height)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

/// <summary>
/// Returns an async function that saves a GenericChart as PNG image
Expand Down Expand Up @@ -175,7 +175,7 @@ module ChartExtensions =
fun (gChart: GenericChart) ->
gChart
|> Chart.savePNGAsync (path, ?EngineType = EngineType, ?Width = Width, ?Height = Height)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

/// <summary>
/// Returns an async function that converts a GenericChart to a SVG string
Expand Down Expand Up @@ -211,7 +211,7 @@ module ChartExtensions =
fun (gChart: GenericChart) ->
gChart
|> Chart.toSVGStringAsync (?EngineType = EngineType, ?Width = Width, ?Height = Height)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

/// <summary>
/// Returns an async function that saves a GenericChart as SVG image
Expand Down Expand Up @@ -251,4 +251,4 @@ module ChartExtensions =
fun (gChart: GenericChart) ->
gChart
|> Chart.saveSVGAsync (path, ?EngineType = EngineType, ?Width = Width, ?Height = Height)
|> Async.RunSynchronously
|> AsyncHelper.taskSync
13 changes: 7 additions & 6 deletions src/Plotly.NET.ImageExport/IGenericChartRenderer.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace Plotly.NET.ImageExport

open System.Threading.Tasks
open Plotly.NET

/// <summary>
Expand All @@ -8,31 +9,31 @@ open Plotly.NET
type IGenericChartRenderer =

///<summary>Async function that returns a base64 encoded string representing the input chart as JPG file with the given width and height</summary>
abstract member RenderJPGAsync: int * int * GenericChart.GenericChart -> Async<string>
abstract member RenderJPGAsync: int * int * GenericChart.GenericChart -> Task<string>
///<summary>Function that returns a base64 encoded string representing the input chart as JPG file with the given width and height</summary>
abstract member RenderJPG: int * int * GenericChart.GenericChart -> string

///<summary>Async function that saves the input chart as JPG file with the given width and height at the given path</summary>
abstract member SaveJPGAsync: string * int * int * GenericChart.GenericChart -> Async<unit>
abstract member SaveJPGAsync: string * int * int * GenericChart.GenericChart -> Task<unit>
///<summary>Function that saves the input chart as JPG file with the given width and height at the given path</summary>
abstract member SaveJPG: string * int * int * GenericChart.GenericChart -> unit

///<summary>Async function that returns a base64 encoded string representing the input chart as PNG file with the given width and height</summary>
abstract member RenderPNGAsync: int * int * GenericChart.GenericChart -> Async<string>
abstract member RenderPNGAsync: int * int * GenericChart.GenericChart -> Task<string>
///<summary>Function that returns a base64 encoded string representing the input chart as PNG file with the given width and height</summary>
abstract member RenderPNG: int * int * GenericChart.GenericChart -> string

///<summary>Async function that saves the input chart as PNG file with the given width and height at the given path</summary>
abstract member SavePNGAsync: string * int * int * GenericChart.GenericChart -> Async<unit>
abstract member SavePNGAsync: string * int * int * GenericChart.GenericChart -> Task<unit>
///<summary>Function that saves the input chart as PNG file with the given width and height at the given path</summary>
abstract member SavePNG: string * int * int * GenericChart.GenericChart -> unit

///<summary>Async function that returns a string representing the input chart as SVG file with the given width and height</summary>
abstract member RenderSVGAsync: int * int * GenericChart.GenericChart -> Async<string>
abstract member RenderSVGAsync: int * int * GenericChart.GenericChart -> Task<string>
///<summary>Function that returns string representing the input chart as SVG file with the given width and height</summary>
abstract member RenderSVG: int * int * GenericChart.GenericChart -> string

///<summary>Async function that saves the input chart as SVG file with the given width and height at the given path</summary>
abstract member SaveSVGAsync: string * int * int * GenericChart.GenericChart -> Async<unit>
abstract member SaveSVGAsync: string * int * int * GenericChart.GenericChart -> Task<unit>
///<summary>Function that saves the input chart as SVG file with the given width and height at the given path</summary>
abstract member SaveSVG: string * int * int * GenericChart.GenericChart -> unit
1 change: 1 addition & 0 deletions src/Plotly.NET.ImageExport/Plotly.NET.ImageExport.fsproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<ItemGroup>
<None Include="..\..\docs\img\logo.png" Pack="true" PackagePath="\" />
<None Include="RELEASE_NOTES.md" />
<Compile Include="AsyncHelper.fs" />
<Compile Include="IGenericChartRenderer.fs" />
<Compile Include="PuppeteerSharpRenderer.fs" />
<Compile Include="ExportEngine.fs" />
Expand Down
48 changes: 21 additions & 27 deletions src/Plotly.NET.ImageExport/PuppeteerSharpRenderer.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
namespace Plotly.NET.ImageExport

open System.Threading
open System.Threading.Tasks
open Plotly.NET
open PuppeteerSharp

Expand All @@ -20,7 +22,7 @@ module PuppeteerSharpRendererOptions =


type PuppeteerSharpRenderer() =

/// adapted from the original C# implementation by @ilyalatt : https://github.com/ilyalatt/Plotly.NET.PuppeteerRenderer
///
/// creates a full screen html site for the given chart
Expand Down Expand Up @@ -61,7 +63,7 @@ type PuppeteerSharpRenderer() =
///
/// attempts to render a chart as static image of the given format with the given dimensions from the given html string
let tryRenderAsync (browser: Browser) (width: int) (height: int) (format: StyleParam.ImageFormat) (html: string) =
async {
task {
let! page = browser.NewPageAsync() |> Async.AwaitTask

try
Expand All @@ -71,41 +73,33 @@ type PuppeteerSharpRenderer() =
return imgStr

finally
page.CloseAsync() |> Async.AwaitTask |> Async.RunSynchronously
page.CloseAsync() |> AsyncHelper.taskSyncUnit
}

/// attempts to render a chart as static image of the given format with the given dimensions from the given html string
let tryRender (browser: Browser) (width: int) (height: int) (format: StyleParam.ImageFormat) (html: string) =
tryRenderAsync browser width height format html |> Async.RunSynchronously

/// Initalizes headless browser
let fetchAndLaunchBrowserAsync () =
async {
task {
match PuppeteerSharpRendererOptions.localBrowserExecutablePath with
| None ->
use browserFetcher = new BrowserFetcher()

let! revision = browserFetcher.DownloadAsync() |> Async.AwaitTask
let! revision = browserFetcher.DownloadAsync()

let launchOptions =
PuppeteerSharpRendererOptions.launchOptions

launchOptions.ExecutablePath <- revision.ExecutablePath

return! Puppeteer.LaunchAsync(launchOptions) |> Async.AwaitTask
return! Puppeteer.LaunchAsync(launchOptions)
| Some p ->
let launchOptions =
PuppeteerSharpRendererOptions.launchOptions

launchOptions.ExecutablePath <- p

return! Puppeteer.LaunchAsync(launchOptions) |> Async.AwaitTask
return! Puppeteer.LaunchAsync(launchOptions)
}

/// Initalizes headless browser
let fetchAndLaunchBrowser () =
fetchAndLaunchBrowserAsync () |> Async.RunSynchronously

/// skips the data type part of the given URI
let skipDataTypeString (base64: string) =
let imgBase64StartIdx =
Expand All @@ -120,7 +114,7 @@ type PuppeteerSharpRenderer() =
interface IGenericChartRenderer with

member this.RenderJPGAsync(width: int, height: int, gChart: GenericChart.GenericChart) =
async {
task {
use! browser = fetchAndLaunchBrowserAsync ()

return! tryRenderAsync browser width height StyleParam.ImageFormat.JPEG (gChart |> toFullScreenHtml)
Expand All @@ -129,10 +123,10 @@ type PuppeteerSharpRenderer() =
member this.RenderJPG(width: int, height: int, gChart: GenericChart.GenericChart) =
(this :> IGenericChartRenderer)
.RenderJPGAsync(width, height, gChart)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

member this.SaveJPGAsync(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
async {
task {
let! rendered =
(this :> IGenericChartRenderer)
.RenderJPGAsync(width, height, gChart)
Expand All @@ -143,10 +137,10 @@ type PuppeteerSharpRenderer() =
member this.SaveJPG(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
(this :> IGenericChartRenderer)
.SaveJPGAsync(path, width, height, gChart)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

member this.RenderPNGAsync(width: int, height: int, gChart: GenericChart.GenericChart) =
async {
task {
use! browser = fetchAndLaunchBrowserAsync ()

return! tryRenderAsync browser width height StyleParam.ImageFormat.PNG (gChart |> toFullScreenHtml)
Expand All @@ -155,10 +149,10 @@ type PuppeteerSharpRenderer() =
member this.RenderPNG(width: int, height: int, gChart: GenericChart.GenericChart) =
(this :> IGenericChartRenderer)
.RenderPNGAsync(width, height, gChart)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

member this.SavePNGAsync(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
async {
task {
let! rendered =
(this :> IGenericChartRenderer)
.RenderPNGAsync(width, height, gChart)
Expand All @@ -169,10 +163,10 @@ type PuppeteerSharpRenderer() =
member this.SavePNG(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
(this :> IGenericChartRenderer)
.SavePNGAsync(path, width, height, gChart)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

member this.RenderSVGAsync(width: int, height: int, gChart: GenericChart.GenericChart) =
async {
task {
use! browser = fetchAndLaunchBrowserAsync ()

let! renderedString =
Expand All @@ -184,10 +178,10 @@ type PuppeteerSharpRenderer() =
member this.RenderSVG(width: int, height: int, gChart: GenericChart.GenericChart) =
(this :> IGenericChartRenderer)
.RenderSVGAsync(width, height, gChart)
|> Async.RunSynchronously
|> AsyncHelper.taskSync

member this.SaveSVGAsync(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
async {
task {
let! rendered =
(this :> IGenericChartRenderer)
.RenderSVGAsync(width, height, gChart)
Expand All @@ -198,4 +192,4 @@ type PuppeteerSharpRenderer() =
member this.SaveSVG(path: string, width: int, height: int, gChart: GenericChart.GenericChart) =
(this :> IGenericChartRenderer)
.SaveSVGAsync(path, width, height, gChart)
|> Async.RunSynchronously
|> AsyncHelper.taskSync
8 changes: 6 additions & 2 deletions tests/Plotly.NET.ImageExport.Tests/ImageExport.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let ``Image export tests`` =
ptestAsync "Chart.toBase64JPGStringAsync" {
let testBase64JPG = readTestFilePlatformSpecific "TestBase64JPG.txt"

let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64JPGStringAsync())
let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64JPGStringAsync() |> Async.AwaitTask)

return
Expect.equal
Expand All @@ -40,13 +40,17 @@ let ``Image export tests`` =
ptestAsync "Chart.toBase64PNGStringAsync" {
let testBase64PNG = readTestFilePlatformSpecific "TestBase64PNG.txt"

let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64PNGStringAsync())
let! actual = (Chart.Point([1.,1.]) |> Chart.toBase64PNGStringAsync() |> Async.AwaitTask)

return
Expect.equal
actual
testBase64PNG
"Invalid base64 string for Chart.toBase64PNGStringAsync"
}
testCase "Chart.toBase64JPGString" <| fun () ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a great idea, as we can at least test if the render engine finishes this way. could you add this for the other image formats as well? maybe rename the tests to Chart.toBase64PNGStringAsync terminates etc

let actual = Chart.Point([1.,1.]) |> Chart.toBase64JPGString()
Expect.isTrue (actual.Length > 100) ""

]
)