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

Add version performance benchmark tool #588

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Conversation

yne
Copy link
Contributor

@yne yne commented Mar 14, 2023

Description

This PR add a per-version performance comparison tool.

Motivation and Context

This tool will help us quantify performance gain/loss between the current /src/ build and the last master build (or any previous version built).

How Has This Been Tested?

Tested on npm start and opening /docs/benchmark.html on chromium

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I did npm run test and all tests passed green (including code styling checks).
  • I have added tests to cover my changes.
  • My change requires a change to the documentation.
  • I have updated the README accordingly.
  • I have read the CONTRIBUTING document.

@yne yne marked this pull request as ready for review March 14, 2023 15:06
@Connum
Copy link
Contributor

Connum commented Mar 17, 2023

Good idea, but I think instead of just parsing a file, we should also let it render some text, because I think that's where we should really see a decrease in performance when more features are added. I'll just leave my test script here that I wrote for testing the forEach()/for() performance:

const textToRender = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789ffffiThe king said: เป็นคนใจดีสำหรับทุกคน because ความรักคือทุกสิ่ง ลลฤๅ'.repeat(10);
const loopCount = 100;

const fileButton = document.getElementById('file');
fileButton.addEventListener('change', onReadFile, false);

let addedTime = 0;
let loopsOutstanding = loopCount;
let errTries = 0;

function onReadFile(e) {
    const file = e.target.files[0];
    const reader = new FileReader();
    reader.onload = function(e) {
        for(let i = 0; i < loopCount; i++) {
            const start = performance.now();
            font = opentype.parse(e.target.result);
            const glyphs = font.stringToGlyphs(textToRender);
            for (let i = 0; i < glyphs.length; i++) {
                const glyph = glyphs[i];
                glyph.path.toSVG();
            }
            const duration = performance.now() - start;
            const msg = 'Execution time: ' + duration + 'ms';
            console.log(msg, loopsOutstanding);
            output.textContent = msg;
            addedTime+=duration;
            loopsOutstanding--;

            if (loopsOutstanding === 0) {
                const finalMsg = 'Average execution time: ' + (addedTime / loopCount) + 'ms';
                console.log(finalMsg);
                output.textContent = finalMsg;
            }
        };
    }
    reader.onerror = function(e) {
        errors.innerHTML += '<br>' + e.currentTarget.error;
        errTries++;
        if (errTries < 100) {
            reader.readAsArrayBuffer(file);
        } else {
            errors.innerHTML += '<br><strong>ABORTED</strong>';
        }
    }
    reader.readAsArrayBuffer(file);
}

@yne
Copy link
Contributor Author

yne commented Mar 17, 2023

I'll add your code to the benchmark 👍

@yne
Copy link
Contributor Author

yne commented Mar 18, 2023

@Connum stringToGlyphs + toSVG added

We can now clearly see a performance drop between

  • 1.3.4 : 1377.9ms
  • master : 5576.9ms (8265.3ms with optimize:true)

@Connum
Copy link
Contributor

Connum commented Mar 18, 2023

Interesting! A lot of features and fixes have been introduced since 1.3.4 and each table support we add increases parsing time. Maybe we should add more options to skip features that may not always be required. We should try to find out which features have the most impact on performance.

@Connum
Copy link
Contributor

Connum commented Nov 15, 2023

I found the culprit for the performance decrease to be roundDecimal() in path.js. This should be improved before releaseing 2.0.0, I'm going to open an issue for this.
This benchmark tool made it way easier to test that, let's add it as-it-is and make improvements with time.

@Connum Connum merged commit 2dbaf7a into opentypejs:master Nov 15, 2023
1 check passed
@yne yne deleted the benchmark branch November 15, 2023 23:19
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.

2 participants