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

More interpreter optimizations #107

Closed
wants to merge 28 commits into from
Closed

More interpreter optimizations #107

wants to merge 28 commits into from

Conversation

Saiv46
Copy link
Contributor

@Saiv46 Saiv46 commented Jun 13, 2020

Closes #69 and #75

Benchmark results by @Karang (benchmark_all_types):

**ProtoDef-io:master**:
read x 26,817 ops/sec ±1.43% (92 runs sampled)
  ✓ reads (5617ms)
write x 16,188 ops/sec ±0.46% (96 runs sampled)
  ✓ writes (5788ms)
read (compiled) x 104,248 ops/sec ±1.56% (87 runs sampled)
  ✓ reads (compiled) (5474ms)
write (compiled) x 79,582 ops/sec ±0.39% (95 runs sampled)
  ✓ writes (compiled) (5616ms)
**Saiv46:master**:
read x 67,847 ops/sec ±0.45% (95 runs sampled)
  ✓ reads (5676ms)
write x 40,270 ops/sec ±0.38% (95 runs sampled)
  ✓ writes (5558ms)
read (compiled) x 102,333 ops/sec ±1.02% (93 runs sampled)
  ✓ reads (compiled) (5524ms)
write (compiled) x 77,432 ops/sec ±1.04% (86 runs sampled)
  ✓ writes (compiled) (5614ms)

Saiv46 added 8 commits June 4, 2020 08:00
I really suck at splitting changes into small commits
* In `compiler.js` I move datatype kind constants into `utils.js`
* Modified `protodef.js`, now interpreter faster up to 2x
* Moved native-compiled datatypes to `shared/` directory
Also, `interpreter.js` now has `DATATYPE_NOCOPY`
This constant could double the interpreter performance.
Gonna seek for bottlenecks
@Saiv46 Saiv46 changed the title Bulk changes More interpreter optimizations Jun 13, 2020
@Saiv46 Saiv46 marked this pull request as ready for review June 13, 2020 15:59
src/interpreter.js Outdated Show resolved Hide resolved
I hope it doesn't hurt performance so mush.
@rom1504
Copy link
Member

rom1504 commented Jun 15, 2020

Can you open a PR for node-minecraft-protocol using the interpreter and this PR as protodef version ?
This would make it possible to check better that this works

@rom1504
Copy link
Member

rom1504 commented Jun 15, 2020

Could you explain what you did here and why it is faster ?

@rom1504
Copy link
Member

rom1504 commented Jan 1, 2021

@Saiv46 do you intend to finish this?

@Saiv46
Copy link
Contributor Author

Saiv46 commented Jan 2, 2021

I need to figure out what's missing here. Something stopped me from finishing this PR... Oh, yes, fields need to be optimized...

@Saiv46
Copy link
Contributor Author

Saiv46 commented Jan 3, 2021

Could you explain what you did here and why it is faster ?

@rom1504 First I was just refactoring existing code - first by just splitting it, then applying some optimizations from protodef-neo.

Recently I read this article about optimization fundamentals - so now I know what to do.

By the way:
(UPD. Benchmarks updated, stable ~3x performance improvement)

-  396 passing (1s)
+  396 passing (464ms)

- C:\Users\Admin\node-protodef>npx mocha ./benchmark/benchmark_all_types.js
+ D:\WORKING\Saiv46\node-protodef>npx mocha ./benchmark/benchmark_all_types.js

- read x 8,250 ops/sec ±2.34% (84 runs sampled)
-   √ reads (6102ms)
+ read x 24,708 ops/sec ±7.08% (85 runs sampled)
+   √ reads (5752ms)
- write x 4,229 ops/sec ±12.03% (76 runs sampled)
-   √ writes (5533ms)
+ write x 12,567 ops/sec ±25.72% (81 runs sampled)
+   √ writes (5635ms)

  4 passing (23s)

@Saiv46
Copy link
Contributor Author

Saiv46 commented Jan 3, 2021

3x interpreter performance improvement achieved!

read x 23,910 ops/sec ±6.52% (80 runs sampled)
  √ reads (6062ms)
write x 12,641 ops/sec ±6.70% (81 runs sampled)
  √ writes (5764ms)

Now I should undo changes to compiler (because this is out of the scope of this PR)
(UPD. Nevermind. Changes revert making compiler slower on benchmarks)

@Saiv46
Copy link
Contributor Author

Saiv46 commented Mar 7, 2021

I was tried to make something more on this PR, but I run out of ideas that will not break anything.

@Saiv46 Saiv46 deleted the branch ProtoDef-io:master September 21, 2021 14:57
@Saiv46 Saiv46 closed this Sep 21, 2021
@Saiv46 Saiv46 deleted the master branch September 21, 2021 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make API methods names clearer : comply to abstract-encoding
3 participants