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

ReportDataAggregator#readTextFrom seems VERY inefficient #228

Closed
transentia opened this issue Apr 11, 2022 · 1 comment
Closed

ReportDataAggregator#readTextFrom seems VERY inefficient #228

transentia opened this issue Apr 11, 2022 · 1 comment

Comments

@transentia
Copy link

I've been trawling through the code (trying to get a handle on AOEpeople/geb-spock-reports#34).

I came across com.athaydes.spockframework.report.internal.ReportDataAggregator#readTextFrom:

    @PackageScope
    static String readTextFrom( RandomAccessFile file ) {
        def buffer = new byte[8]
        def result = new StringBuilder( file.length() as int )

        int bytesRead
        while ( ( bytesRead = file.read( buffer ) ) > 0 ) {
            result.append( new String( buffer[ 0..( bytesRead - 1 ) ] as byte[], charset ) )
        }
        return result.toString()
    }

Reading 8 bytes at a time and making and appending lots of tiny strings seems really inefficient! Is there perhaps a micro-optimisation that can be done here? Something LIKE:

    static String readTextFrom( RandomAccessFile file ) {
        def buffer = new byte[file.length() as int]

        int bytesRead = file.read(buffer);
        if (bytesRead != buffer.length) {
            // error message or something...
        }

        return new String(buffer, 0, bytesRead, charset)
    }

...this specific code would fail if given a long-sized file, but that's unlikely (? famous last words)?

@renatoathaydes
Copy link
Owner

I agree, this buffer is too small, and the allocation of a String inside the loop is unnecessary.
Do you want to send a PR fixing that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants