feat: release GIL when possible

[JonAnCla]

Hi there, I was trying to use multiple threads to simultaneously extract many excel files and didn't see the all cores being used, so suspected that the GIL was not being released within fastexcel

I've added a GIL-releasing closure in two places as noted in the title: rust implementation of read_excel and ExcelReader.build_sheet

With these in place, I see all cores being used in a multi-threaded setup :)

I expect more could be done to release the GIL for longer, but I think I've covered the two major places where most time is spent without needing a wider refactor

Also apologies for confusion, commits have been made under the user jc-5s which is also mine (work account)

Thanks!

[arnabanimesh]

How much reduction in read times are you getting?

[JonAnCla]

About 3-4x improvement Vs current 0.15.1, in both cases reading 150 files using concurrent.futures ThreadPoolExecutor

I'm not sure how much a difference it makes but I did not make a production build with profiling for my "release Gil" build, so it could be a bit faster with that added on top

[JonAnCla]

Also just to add, without this change I was seeing about 20% total usage across all cores for the duration of reading the 150 files. With this change I see 100% usage across all cores for the duration

[arnabanimesh]

I guess you could use ProcessPoolExecutor. But this is fine too.

I am a bit worried about the overhead due to context switching when running multiple threads on a single core when using ThreadPoolExecutor in free threaded python builds. But I guess the overhead will be negligible since CPython devs are smart enough and should have considered this scenario.

[JonAnCla]

I guess you could use ProcessPoolExecutor. But this is fine too.

Threads have lower startup, memory and communication overhead, but need the GIL to be released to bring true parallelism in python (pre nogil)

I am a bit worried about the overhead due to context switching when running multiple threads on a single core when using ThreadPoolExecutor in free threaded python builds. But I guess the overhead will be negligible since CPython devs are smart enough and should have considered this scenario.

I agree it should be low overhead, there's some slightly relevant text here: https://pyo3.rs/v0.25.1/free-threading.html#many-symbols-exposed-by-pyo3-have-gil-in-the-name. In "with GIL" python the thread holding the GIL can switch every ~5ms so I assume switching has been fairly well tuned and any logic still needed carried over to nogil python

[JonAnCla]

In this section of same doc linked above, its noted that its still important to detach threads doing long running-work from the python interpreter (even in nogil python), so that they do not block global synchronisation i.e. "stop the world" events like Garbage Collection https://pyo3.rs/v0.25.1/free-threading.html#global-synchronization-events-can-cause-hangs-and-deadlocks

So adding the the GIL releasing code would actually help tie in with nogil python too :)

[lukapeschke]

@hottwaj Thank you, great idea! I think it should also be added to lazy code paths though. Also, could you please provide your benchmark script ?

[jc-5s]

Great thanks, I've added a change that also releases the GIL in the "eager" case for ExcelReader.build_sheet (I had done the "lazy" case in first set of changes). I'm not able to see how to make similar changes to the "load/build_table" methods though - do they ultimately rely on build_sheet?

the test I've been running is quite basic - see below. you can use a list of files or open the same file many times in parallel.

• reading the files sequentially using openpyxl took about 160s
• reading them sequentially with fastexcel was about 22s
• reading them in multiple threads with fastexcel 0.15.1 was about 11s (so there is currently some multi-threading benefit)
• reading them in multiple threads with the GIL-releasing changes was about 2.7s

import fastexcel
import os
from concurrent.futures import ThreadPoolExecutor

def process_file(filepath):
    reader = fastexcel.read_excel(filepath)
    reader.load_sheet(0)

with ThreadPoolExecutor() as executor:
    results = list(executor.map(process_file, files_list))

[JonAnCla]

sorry for confusing things, that was me - keep forgetting to switch accounts

[lukapeschke]

Thank you for the script and the updates :) I've left a comment regarding worksheet_range_ref.

Regarding the table loading functions, I guess you could wrap the call to load_table here , since this method is working on owned rust types

[JonAnCla]

Great have made further changes that hopefully cover those points. LMK if anything else, thanks!

[JonAnCla]

Hi there just checking if there's anything else needed from me at the moment? the PR seems to say changes have been requested, but they seem to already been committed @lukapeschke and I'm fine with them, so unless I've missed something I think this is waiting on maintainers? LMK if I'm missing something, thanks!

[lukapeschke]

@JonAnCla I ran a few benchmarks and released the GIL in a few more places. Most low-hanging fruits regarding gil-related perf should have been adressed, I'll merge once the CI passes. Thanks again for your contribution! 🙂

[JonAnCla]

Nice, thanks!

[lukapeschke]

My measurements are available here: #397 (comment)