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.

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

First cut for Miso (GHCJS) benchmark #223

Closed
wants to merge 14 commits into from

Conversation

saurabhnanda
Copy link

@saurabhnanda
Copy link
Author

Could you please help me debug the following error:

benchmarking  { name: 'miso-ghcjs-keyed',
  keyed: true,
  uri: 'miso-ghcjs-keyed',
  useShadowRoot: false } 08_create1k-after10k
after initialized  08_create1k-after10k 0 miso-ghcjs-keyed
not found
REJECTED PROMISE TypeError: Cannot read property 'findElements' of null
    at n.then.nd (/Users/saurabhnanda/projects/js-framework-benchmark/webdriver-ts/dist/webdriverAccess.js:38:28)
    at ManagedPromise.invokeCallback_ (/Users/saurabhnanda/projects/js-framework-benchmark/webdriver-ts/node_modules/selenium-webdriver/lib/promise.js:1384:14)
    at TaskQueue.execute_ (/Users/saurabhnanda/projects/js-framework-benchmark/webdriver-ts/node_modules/selenium-webdriver/lib/promise.js:3092:14)
    at TaskQueue.executeNext_ (/Users/saurabhnanda/projects/js-framework-benchmark/webdriver-ts/node_modules/selenium-webdriver/lib/promise.js:3075:27)
    at asyncRun (/Users/saurabhnanda/projects/js-framework-benchmark/webdriver-ts/node_modules/selenium-webdriver/lib/promise.js:2935:27)
    at /Users/saurabhnanda/projects/js-framework-benchmark/webdriver-ts/node_modules/selenium-webdriver/lib/promise.js:676:7
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

When I'm appending 1,000 rows after creating 10,000 rows manually (i.e. outside the webdriver benchmark) it seems to be working. However, this is the only benchmark that is consistently failing when run with webdriver.

I have added the compiled JS files in dist/all.js to save you from the hassle of installing the base GHCJS system from scratch. You should be able to access the benchmark via http://localhost:8080/miso-ghcjs-keyed/ normally.

@dmjio
Copy link
Contributor

dmjio commented Aug 1, 2017

Author here, there needs to be some updates to this PR before I'd say it should be cleared for merge. Also, I'm not sure how @krausest feels about adding all.js to this repo (1MB+) of js. Using nix one could get all.js w/o building GHCJS. Let's rethink this @saurabhnanda.

cc @krausest

@NickSeagull
Copy link

I feel really excited that Haskell libraries are getting more diffussion, although I've looked at the source code of the benchmark and it uses the String type (antique type that Haskell still uses for backwards compatibility) instead of Text which is the current standard in the Haskell ecosystem, or even better, MisoString, a specialized string type from miso, optimized for the web. I think that it is not fair to base these conclusions in an old standard.

Also, I think it is better to provide all.js, which is basically the Haskell runtime compiled to JS, in a different way, like a script that downloads it from a git repo or something... I don't know, just giving ideas 😁

Create 1,000 rows - Miso 1.9x slower than Elm
Replace 1,000 rows - Miso 1.4x FASTER than Elm
Update every 10th row - Miso 3.7x slower than Elm
Select row - Miso 4.8x slower than Elm
Swap row - Miso 2.9x slower than Elm
Remove row - Miso 1.9x slower than Elm
Create 10,000 rows - Miso 1.5x slower than Elm
Append rows - Miso 2.8x slower than Elm
Clear 10,000 rows - Miso at-par with Elm
Startup time - Miso 2.6x slower than Elm
Create 1,000 rows - Miso 1.9x slower than Elm (as opposed to 1.9x slower)
Create 10,000 rows - Miso 1.6x slower than Elm (as opposed to 1.5x slower)
Append rows - Miso 2.8x slower than Elm (as opposed to 2.8x slower)
@krausest
Copy link
Owner

krausest commented Aug 2, 2017

I'm pretty concerned with adding more prerequisites besides java (and scala, wich is already very heavy, but much lighter than installing the haskell tool stack which installed ~ 4GB in my home directory).

I could imagine adding the compiled javascript files for languages that have such a heavy compilation process and make the build in such cases optional (something like a build-disabled-by-default script task in package.json).

Would you be willing to create pull requests when major updates happen to miso or ghcjs? I think I'd have to create a deprecation policy for "binary-only" frameworks in case no updates happen - because I'm afraid I won't be able to maintain those non-javascript-frameworks for non trivial updates.

Could you please rename the miso-ghcjs folder to include a version number?

@saurabhnanda
Benchmark 08 runs fine for me:
result miso-ghcjs-keyed_08_create1k-after10k.json 708.167 1034.779 758.1183 66.87093118171751
Do you still see the problem?

@dmjio
Copy link
Contributor

dmjio commented Aug 3, 2017

@krausest @saurabhnanda, I'd rather not commit these benchmarks until I've finished implementing the child keys patch, since they won't be accurate, and will give people a faulty understanding of this framework's abilities. Can we please close this PR? I think writing the benchmark code would be better left up to the author. @saurabhnanda did not ask for my permission or coordinate with me on this.

@leeoniya
Copy link
Contributor

leeoniya commented Aug 3, 2017

@dmjio

will give people a faulty understanding of this framework's abilities

don't feel offended. there are plenty of impls here not written by framework authors but by users without any blessings. if the impl is sub-optimal, then submit a faster or more idiomatic impl. if the framework is outdated, then submit an update.

fyi, no matter how fast the impl ends up being, if it requires parsing 1.5MB of GHCJS first, it's not going to do well here.

I think writing the benchmark code would be better left up to the author

if only the author can write fast code in the framework, won't that give average users a faulty expectation of their code's performance? ;p...generally this is true, though.

@dmjio
Copy link
Contributor

dmjio commented Aug 3, 2017

Benchmark code is typically contrived to be optimal for a specific set of assumptions. Also, creating optimal Haskell is not always trivial. But very basic things are missing or lacking entirely in this example. You guys do what you want. I'll just come back to it later

@leeoniya
Copy link
Contributor

leeoniya commented Aug 3, 2017

You guys do what you want. I'll just come back to it later

sure, i wasn't suggesting that this should be merged.

@saurabhnanda
Copy link
Author

Would you be willing to create pull requests when major updates happen to miso or ghcjs?

@krausest wouldn't this be the case for regular JS frameworks as well? Aren't the versions of the library and all its dependencies locked? Isn't there a high chance that the build would fail if newer versions of dependencies are automatically fetched (any lib could introduce a backward incompatible change!)

Could you please rename the miso-ghcjs folder to include a version number?

Sure, I'll do that.

Benchmark 08 runs fine for me:
result miso-ghcjs-keyed_08_create1k-after10k.json 708.167 1034.779 758.1183 66.87093118171751Do you still see the problem?

No, it's working now. The test was timing-out earlier, but @dmjio made some suggestions which improved the spreed and its working for me now.

Benchmark code is typically contrived to be optimal for a specific set of assumptions.

I have a different philosophy for benchmarking, ie user, not authors should write them. And when I'm writing benchmarking code I tend to write the most obvious thing first - right after reading a tutorial. Unless it's doing something really stupid (like using widely known performance killers, eg fold' vs foldl), I'd like to stick to real-life code (as opposed to highly tweaked code, written to game the benchmark).

At the end, it's up to you @krausest Have other frameworks submitted highly tweaked code? What it your preference as the maintainer - real life vs tweaked?

@krausest I can also understand and appreciate how @dmjio is feeling about this, given that he's putting a lot of hard work into a brand new framework, which obviously has a lot of things in WIP mode. Given the marketing impact these benchmarking numbers have, it definitely runs the danger of putting off early adopters. May I request you to have the ability to add notes along with each framework's results, which allow the author to explain the slowdown and link to specific Github issues addressing them. This, I believe, balances the needs of users and concerns of authors.

As a user, I'd like to get some idea about the performance of a framework today (with some caveats and notes) rather than being left in the dark. Let me make an informed decision.

@dmjio
Copy link
Contributor

dmjio commented Aug 3, 2017

@saurabhnanda

May I request you to have the ability to add notes along with each framework's results, which allow the author to explain the slowdown and link to specific Github issues addressing them.

First of all, this isn't an accurate benchmark because the keys patch isn't finished, yet you've incorrectly named this miso-ghcjs-keyed, please change this.

Furthermore, the code you've submitted is performing updates to the model in IO, which isn't what a typical user would do (but is a tweak -- to refute your earlier point). The elm code doesn't do this either. Your initial usage of String was also very worrisome.

Lastly, you're not collaborative. I've asked you to join the slack and talk with the others, but you refuse, and now you're trying to submit faulty benchmarks against my better wishes. Not a great start.

@saurabhnanda
Copy link
Author

First of all, this isn't an accurate benchmark because the keys patch isn't finished, yet you've incorrectly named this miso-ghcjs-keyed, please change this.

Right. I'll change that.

Furthermore, the code you've submitted is performing updates to the model in IO, which isn't what a typical user would do (but is a tweak -- to refute your earlier point). The elm code doesn't do this either. Your initial usage of String was also very worrisome.

that's the most natural way that occurred to me after staring long and hard at the Random API and Vector API in Haskell. I tried your suggestion of using a non-IO random generator, but it has equivalent performance. If you'd prefer using the non IO version, then I already have the code available - vacationlabs@dd08f0b Let me know and I'll change it.

Lastly, you're not collaborative. I've asked you to join the slack and talk with the others, but you refuse, and now you're trying to submit faulty benchmarks against my better wishes. Not a great start.

I concede on not using slack/gitter (most of the chat doesn't get indexed by Google, and I strongly feel a lot of these conversations should get indexed and be available for the benefit of others). But I wouldn't say this is a shoot and scoot operation that I'm attempting here, don't you think - dmjio/miso#225 ? Anyway, I'd suggest just agreeing to disagree on whether submitting benchmarks need the blessing of the framework author. I'm still available in good faith to make the benchmark more representative if you still feel if it's out of whack. Heck we can even call it miso-non-official if @krausest is fine with that.

@dmjio
Copy link
Contributor

dmjio commented Aug 3, 2017

that's the most natural way that occurred to me after staring long and hard at the Random API and Vector API in Haskell. I tried your suggestion of using a non-IO random generator, but it has equivalent performance. If you'd prefer using the non IO version, then I already have the code available - vacationlabs/js-framework-benchmark@dd08f0b Let me know and I'll change it.

Use the FFI version from Math.random(). This is what elm's random gen is based on.

I concede on not using slack/gitter (most of the chat doesn't get indexed by Google, and I strongly feel a lot of these conversations should get indexed and be available for the benefit of others)

You're opinion on wether or not slack gets indexed isn't an excuse to not collaborate, we also have #haskell-miso IRC.

Anyway, I'd suggest just agreeing to disagree on whether submitting benchmarks need the blessing of the framework author.

I'd gladly work with anyone who knew no Haskell, but was willing to collaborate. As opposed to someone who knows some Haskell but won't collaborate.

But I wouldn't say this is a shoot and scoot operation that I'm attempting here, don't you think

That's exactly what this is. You really aren't sure about what you're doing, and you're just trying to run this through.

@saurabhnanda
Copy link
Author

I concede on not using slack/gitter (most of the chat doesn't get indexed by Google, and I strongly feel a lot of these conversations should get indexed and be available for the benefit of others)

You're opinion on wether or not slack gets indexed isn't an excuse to not collaborate, we also have #haskell-miso IRC.

Anyway, I'd suggest just agreeing to disagree on whether submitting benchmarks need the blessing of the framework author.

I'd gladly work with anyone who knew no Haskell, but was willing to collaborate. As opposed to someone who knows some Haskell but won't collaborate.

But I wouldn't say this is a shoot and scoot operation that I'm attempting here, don't you think

That's exactly what this is. You really aren't sure about what you're doing, and you're just trying to run this through.

I'll respond to all this with a cooler head in some time (and probably directly to @dmjio). This line & tone of conversation is not adding value to anyone.

Wrt whether the benchmark should be merged, or not, I'll leave it up to @krausest I'm available to make changes that seem appropriate.

Use the FFI version from Math.random(). This is what elm's random gen is based on.

Btw, this doesn't seem appropriate to me, because this is an optimisation that GHCJS should be doing directly, IMO. If these kind of tweaks/hacks are required to speed up Miso/GHCJS for the benchmark then I'd suggest having two versions of the benchmark code miso-vanilla and miso-performance. Looking at the diff between the two codebases will give people a good idea of what one would need to do, to extract performance from critical pieces of code, when required in their project.

@dmjio
Copy link
Contributor

dmjio commented Aug 3, 2017

I'll respond to all this with a cooler head in some time (and probably directly to @dmjio). This line & tone of conversation is not adding value to anyone.

Neither is your lack of willingness to collaborate with the others.

Wrt whether the benchmark should be merged, or not, I'll leave it up to @krausest I'm available to make changes that seem appropriate.

@saurabhnanda, regardless of the merge I'll fork and host my own as the official. I've tried, but it's obvious we can't collaborate.

@saurabhnanda
Copy link
Author

Neither is your lack of willingness to collaborate with the others.

I'm sorry, but I disagree. Not having a conversation, specifically on slack, is equivalent to not being collaborative? Responding to every comment on the relevant Github issue, and trying all suggestions and posting all interim results - what would you call that?

@dmjio I'm rooting for Miso, btw. I just feel strongly about benchmarks being made available ASAP and being made better with every release of the library.

Over and out from my side on this particular side of the conversation. Rest I'll handle privately with @dmjio

Again, I'm still available to work on this PR to get it merged upstream. It would be unfortunate if this (or some other version) didn't get merged because of a meta-discussion.

Doesn’t seem to be making anything better:

Create 1,000 rows - Miso 1.9x slower than Elm
Replace 1,000 rows - Miso 1.2x FASTER than Elm
Update every 10th row - Miso 3x slower than Elm
Select row - Miso 5.7x slower than Elm
Swap row - Miso 3.3x slower than Elm
Remove row - Miso 1.4x slower than Elm
Create 10,000 rows - Miso 1.6x slower than Elm
Append rows - Miso 2.9x slower than Elm
Clear 10,000 rows - Miso at-par with Elm
Startup time - Miso 2.7x slower than Elm
@saurabhnanda
Copy link
Author

@krausest Hopefully, my final set of changes:

  • Changed the name of the benchmark to miso-non-official-v0.4.0.0-non-keyed
  • Compiled the final JS file with closure compiler (but unable to use ADVANCED_OPTIMISATIONS due to some GHCJS bug - probably). The benchmarks on my local machine show that there is some improvement, but I'm not 100% certain. I've left all.js and all.min.js in the dist folder if you'd like to take a look.

Let me know if something needs to be changed to make this a "non buildable" benchmark.

I hope this gets merged. We need more visibility for Haskell JS frameworks.

Thanks @krausest for maintaining these benchmarks. And thanks to @dmjio for the Miso framework.

@krausest
Copy link
Owner

krausest commented Aug 3, 2017

You guys are giving me a bad headache ;-)
Haven't run into that situation before.

@saurabhnanda Thanks for your effort!
Still I've come to the conclusion that I'm not going to merge or publish the results if the author of the framework objects to doing so. It would harm the benchmark collection if I exposed it to criticism from framework authors.

I'm not sure how I feel about miso. Seems like it was either too early for the framework or too difficult for its users to create a reasonable implementation. I implemented a few benchmarks for javascript frameworks and with one exception their frameworks authors didn't make a riot (pun intended) and I guess that's part of what a developer should be allowed to ask from a framework (small improvements are fine, but you shouldn't be far off - that's the "frame" in the framework).

I'm still not sure how I'm going to handle frameworks that have such big prerequisites. I'm currently pretty happy that I have a circleci build that compiles and runs all frameworks. With a dependency like ghcjs things would be much harder. And it would harm acceptance for developers trying to build the benchmarks on their own machine - already now people are complaining about the bloat (and they didn't get those 5.8 GB in .ghcjs and .stack yet).
I'm really thinking about setting up a policy requiring a "binary" (transpiled javascript) version to be committed in such cases and ask the contributor to update the benchmark when needed. I can (and do) handle updates for javascript frameworks in most cases, but I can't promise that for haskell.

@alexfmpe
Copy link
Contributor

alexfmpe commented Aug 3, 2017

I'm really thinking about setting up a policy requiring a "binary" (transpiled javascript) version to be committed in such cases and ask the contributor to update the benchmark when needed. I can (and do) handle updates for javascript frameworks in most cases, but I can't promise that for haskell.

Not trying to hijack the thread, but I'll leave my two cents here, since this concern is relevant to the benchmark I'm working on: reflex-dom (also Haskell by way of ghcjs). There's also at least one purescript benchmark on the way, so I'm afraid we already opened pandora's box.

How about requiring that npm run build-prod deals only with javascript/typescript code (either as source or compilation target), and we add, say, npm run build-prod-transpile, which compiles the source people actually work on, but signalises that that mode of operation is not supported by the main repo maintainers, and any problems are the responsibility of the package maintainer. Not sure where to place elm/scala here though.

We could always take this one step further and allow choosing exactly what languages are to be installed/built/tested (default: javascript/typescript), the same way one can choose for which frameworks benchmarks are run.

There's also the nuclear option: have a separate repo for Haskell frameworks that is then included into this one as a submodule. That repo could internally have its own rules (cabal/stack/nix) and its own CI, and somehow export compiled javascript (e.g. 'binary' releases, continuous delivery branch, magical dwarves), that is compatible with the main one.
That way, @krausest only needs to worry about consuming javascript-ish stuff, and the Haskell guys only need to worry about producing javascript.

Usually I prefer to keep things that need to work together in the same repo, so they all go in the same CI and don't break each other, but y'know, tradeoffs.

@thomashoneyman
Copy link
Contributor

thomashoneyman commented Aug 3, 2017

To follow on from @alexteves, I have put together benchmarks for Pux and Halogen, two of the three primary frameworks used in PureScript.

There is growing interest in benchmarks for these PureScript frameworks, so I've created implementations based on your repository (thanks for the great work!). They're a little different from the Haskell ones above because PureScript is much closer to Elm.

In the Haskell, Elm, and PureScript cases, each one is a functional language compiled to JavaScript, and perhaps it does make sense to have that be a subset of these benchmarks rather than mixed in.

Of course, that's your decision @krausest, but I thought I'd let you know about other functional benchmarks that have been put together.

Cheers,
Thomas

@alexfmpe
Copy link
Contributor

alexfmpe commented Aug 3, 2017

Regarding the whole idiomatic vs performant implementation: do we really have to choose?
I think one could have both implementations in the same subdirectory, with a different index.html for each, and then add two entries in webdriver-ts/src/common.ts.

To me this seems more significant the farther away from vanillajs we go (Sufficiently Smart Compiler and all that jazz), so it might only really affect a handful of frameworks.
The greatest drawback I see is having the full suite take even longer to run. I'm assuming the build requirements are pretty much the same across variants.

@leeoniya
Copy link
Contributor

leeoniya commented Aug 3, 2017

Regarding the whole idiomatic vs performant implementation: do we really have to choose?
I think one could have both implementations in the same subdirectory, with a different index.html for each, and then add two entries in webdriver-ts/src/common.ts.

personally, i'd also like there to be both perf-tuned and idiomatic impls (if they differ), though not all in the same table. a third table that compares any frameworks with both variants would help cut down the clutter. i'm not terribly thrilled that there are already a whole bunch of React impls each with a different state management lib - each slower than the bare React impl.

i've had the benchmark vs stress-test discussion in other forums before. displaying 10k rows, appending 1k rows and clearing 10k rows certainly falls into the stress test category that would be suitable for perf-tuned code, because this is an extreme case you would expect users to optimize for if for some reason it was actually necessary. the other metrics are more "real world" and would work well for idiomatic impls. most complex/heavy websites are < 3,500 dom nodes in total. Gmail's [pretty bloated] inbox has 3,874. try it out in the console on your favorite bloated property: document.querySelectorAll("*").length. by comparison, the 10k list metric in this bench has 80,030 dom nodes - it is literally insane.

by having a single table, you're forced to choose if you want fast-but-less-clear code or most-obvious-but-slow-10k code. most authors choose the former.

@saurabhnanda
Copy link
Author

Still I've come to the conclusion that I'm not going to merge or publish the results if the author of the framework objects to doing so. It would harm the benchmark collection if I exposed it to criticism from framework authors.

Sorry about the noise. Didn't expect that to happen.

I'm really thinking about setting up a policy requiring a "binary" (transpiled javascript) version to be committed in such cases and ask the contributor to update the benchmark when needed.

  • Binary along with the source code
  • regular install/build commands are skipped or are no-ops
  • a special install/build command is there for anyone who wants to actually recompile the binary from source

@saurabhnanda
Copy link
Author

On perf vs idiomatic, as a user I'd like to see the difference in performance vs idiomatic and have the code available to me to understand how to do performance tuning specific to that framework.

@krausest
Copy link
Owner

krausest commented Aug 5, 2017

@SayLU PureScript seems to be a "bit" more lightweight with ~64 MB diskspace and npm based installation. Thus I'm looking forward to getting a pull request.

@krausest
Copy link
Owner

krausest commented Aug 5, 2017

The question of maintenance depends on how close the implementations stay to javascript, not in terms of the language, but in terms of the ecosystem. Haskell seems to be particularly far off (both haskell based frameworks had spectacular dependencies), elm and probably PureScript too are close enough such that I can promise basic maintenance.

@krausest
Copy link
Owner

krausest commented Aug 8, 2017

Closing this PR.

@krausest krausest closed this Aug 8, 2017
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.

None yet

7 participants