compress base64 buffers - #14485
Conversation
|
Just noting that the JS tests that start from base64 encoded expected values were a bit of a pain to update. I had to manually run the values though this little python helper and copy/paste the results back into the tests: import gzip
from base64 import b64encode, b64decode
def refutz(data):
raw = b64decode(data)
compressed = gzip.compress(raw, mtime=0)
return b64encode(compressed)It might be nice to start from actual array/image data in the tests but I guess that's not possible in all cases, e.g. JSON recordings for "cross" tests. |
|
Hrm, these tests are passing locally for me: Perusing the Gzip RFC it seems as though an "OS" field is encoded in the header, which I will speculate is causing the difference here. So it looks like those tests will have to be handled a little more carefully... Since these are on the python side it might just be easier to round-trip everything for comparison, rather than comparing the encoded values directly. Edit: really strange/interesting, OSX tests for 3.11 and 3.12 pass, which is consistent with my running locally on OSX. But OSX for 3.13 is not passing. Definitely seems like all these will just have to round-trip everything. |
| encoded_bytes = b64encode(gzip.compress(val["key"], mtime=0)).decode("utf-8") | ||
|
|
||
| assert serialize_json(rep.content) == f"""\ | ||
| {{"type":"map","entries":[["key",{{"type":"bytes","data":"{encoded_bytes}"}}]]}}\ |
There was a problem hiding this comment.
Noting that I split the binary content out into its own test. Trying to round-trip in the "mega test" just became unreadable. The big test can continue to check all the features of the serializer at the top level (content, pretty-printing, etc) and this test can maintain the basic encoding for bytes.
|
@mattpap the last changes leave some cross tests failing for the above mentioned reasons. What do you want to do here? I simply don't think we can store static JSON in cases like this, given how the gzip header varies by platform. Edit: Well looking at the test results I am not sure is is just the platform that is the distinguishing factor. But there is something causing header differences that the tests will need to account for or ignore. |
|
Neat! Would be great to see some performance number in terms of space saving and overhead for compression and decompression. Pretty sure it's worth it but would love to have real numbers. |
|
@bokeh/dev Here's a quick benchmarking script Detailsimport argparse
import importlib
from time import time
import pandas as pd
import bokeh
from bokeh.embed import file_html
from bokeh.plotting import figure
# assume 3.8 is local build with compression
compression = "with" if bokeh.__version__.startswith("3.8.0.dev") else "without"
def run(func, N):
img = func(N)
t0 = time()
p = figure(width=400, height=400)
p.image_rgba(image=[img], x=0, y=0, dw=10, dh=10)
out = file_html(p)
t1 = time()
dt = (t1 - t0)
size = len(out.encode("utf-8"))
return (dt, size)
def benchmark(module, course):
name = module.__name__
rows = []
print(f"benchmarking {name} {compression} compression")
for i in course:
print(f"running for N={i}")
result = run(module.func, i)
rows.append((name, compression, i, *result))
columns = ("name", "compression", "N", "time", "size")
return pd.DataFrame(rows, columns=columns)
if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument('module', type=str, help='module to benchmark')
parser.add_argument('--start', type=int, help='bench range start', default=50)
parser.add_argument('--stop', type=int, help='bench range stop', default=3000)
parser.add_argument('--step', type=int, help='bench range step', default=50)
args = parser.parse_args()
module = importlib.import_module(args.module)
course = range(args.start, args.stop+1, args.step)
df = benchmark(module, course)
print(df)
filename = f"{module.__name__}_{compression}_{args.start}_{args.stop}_{args.step}"
df.to_csv(filename, index=False)Next define a module containing a single function import numpy as np
def func(N):
img = np.empty((N,N), dtype=np.uint32)
view = img.view(dtype=np.uint8).reshape((N, N, 4))
for i in range(N):
for j in range(N):
view[i, j, 0] = int(i/N*255)
view[i, j, 1] = 158
view[i, j, 2] = int(j/N*255)
view[i, j, 3] = 255
return imgThen run like this passing the module name and any start/stop/step values you want: python benchmark.py ramp --step 500that will print t dataframe with the results and also write a CSV with the results that can be combined e.g. for plotting. Naturally, you will need to run the benchmark script both with a local build of this PR branch, and with some older Bokeh version in order to obtain comparable results. Note that the timing is inclusive of this set of operations specifically: i.e. it does not include costs for the image generation or any file i/o. For now, here are some quick results with the invocation above for with/without compression: without compressionwith compressionAs you can see for this particular case there is almost two orders of magnitude improvement in size for larger images, and at most a ~2x slowdown for the very small images. That said, I can believe the particular nature of this I should add: I have no idea how to benchmark on the BokehJS side. Someone else will have to do that if they really desire it. I think the results from this kind of benchmark will already make a clear case for inclusion regardless, however. |
I would be surprised if such differences can't be eliminated. This would be a nightmare for computing checksums, among other things. However, if we can't eliminate these, then I would disable compression in cross tests, which should be an option regardless in my opinion. |
|
Here are some quick results with a random image (two channels replaced with random ints) # rand.py
import numpy as np
from random import randint
def func(N):
img = np.empty((N,N), dtype=np.uint32)
view = img.view(dtype=np.uint8).reshape((N, N, 4))
for i in range(N):
for j in range(N):
view[i, j, 0] = int(i/N*255)
view[i, j, 1] = 158
view[i, j, 2] = randint(0, 255)
view[i, j, 3] = randint(0, 255)
return imgwithout compressionwith compressionAs expected, significantly less space savings as well as more expensive in terms of time execution with random data. So, what is really needed is some image gen functions that produce image that are in between these two extremes, and more representative of real-world images. I'd appreciate any help with this as my time is a bit limited. Also note I did not tinker with whatever the default compression level is. I suppose it might be reasonable to make compression level and even use of compression configurable (though I am less sure about that last one). |
The Gzip RFC specifies (at least) a field to encode OS (e.g. Unix, or... Amiga!) and I didn't see anything in the various APIs analogous to setting
I'm on the fence about this, tbh. I think testing for [1] base64 encoded gzipped data will always start with |
|
some results for a without compressionwith compressionI ran several more Also, I checked and the default compression level is 9 already. |
|
@mattpap thinking more, I'd really only like to expose the compression level as a configurable option. That's because import gzip
from base64 import b64encode
val = bytes([0xFF, 0x00, 0x17, 0xFE, 0x00])
b64encode(gzip.compress(val, compresslevel=0, mtime=0))Some results are: Backing out to the un-b64encoded values, it is indeed the OS field that is different: In [15]: osx_zipped[9]
Out[15]: 19
In [16]: unix_zipped[9]
Out[16]: 3I don't think the Python
So AFAICT it would be permissible for us to just unconditionally "fix up" the OS field value to be 255 on our end before base64 encoding. But I think that would necessary involve copying, and the overhead for that would probably not be tolerable. I don't want to add muddled UX and unnecessary implementation complexity just to appease two tests. If you have any other suggestions, I would love to hear them. Otherwise I think the options are to re-work these two tests as non-cross tests, or to make a way for cross tests to know about and ignore this difference for encoded buffers. Edit: I guess will have to dig further. According to the actual cpython source code, the OS field is supposed to be set to 255 always: but that is clearly not what I am seeing in actual results running the same code on different platforms and looking at the 10th byte... |
|
@bryevdv, git blame shows it is a relative new fix for python/cpython#112346 |
|
nice find @hoxbro what a mess! OTOH maybe this points to a quick and dumb solution:
Then: 🎉 |
c1765aa to
ea73280
Compare
ea73280 to
5918669
Compare
Was the native Compression Streams API a possible consideration? This blog does a comparison of various libs and provides a site to run your own benchmarks. The API does not seem to provide a way to set |
|
@febkor we could probably get away without setting That said, this is the PR I have the time to make right now. So, someone else could look at replacing |
|
Thanks for the response!
Of course, please don't delay this PR for a native API implementation. |
ianthomas23
left a comment
There was a problem hiding this comment.
I like this, and am happy to accept it as it is with the performance vs size tradeoffs and the default compression level.
I noticed there could be some simplification replacing loops with TextEncoder and TextDecoder APIs, but that is code that has been around for years and not really changed by this PR so it should probably be considered a separate issue.
| import {gunzipSync, gzipSync} from "fflate" | ||
|
|
||
| export function b64encode(data: Uint8Array): string { | ||
| const chars = Array.from(data).map((b) => String.fromCharCode(b)) |
There was a problem hiding this comment.
Could change this and the next line to
const chars = new TextDecoder().decode(data)
return btoa(chars)There was a problem hiding this comment.
@ianthomas23 I did try this but couldn't get it to work. Looking at the docs for these I am not sure TextEncoder and TextDecoder do base64 encoding and decoding, specifically. There are apparently some Uint8Array APIs for base64 but they do not appear to be widely available.
There was a problem hiding this comment.
Yes, one would still need to use btoa and atob as well.
| export function base64_to_buffer(base64: string): ArrayBuffer { | ||
| const binary_string = atob(base64) | ||
| export function b64decode(data: string): Uint8Array { | ||
| const binary_string = atob(data) |
There was a problem hiding this comment.
Could replace the following 5 lines with the one-liner
const bytes = new TextEncoder().encode(binary_string)|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR adds support to transparently compress and decompress any data that would currently be transmitted as a base64 buffer.
In order to easily support synchronous gzip compress and uncompress on the BokehJS side,
fflatewas added as a dependency.