Super slow ByT5 Tokenizer
Author: PhilipMay
Pull request: no

Hi,

The ByT5 Tokenizer seems to be super slow compared to others (T5).
See colab link below for example code.

T5-small tokenizer:

![grafik](https://user-images.githubusercontent.com/229382/126946873-9e2cc68c-409f-4428-ae90-b08469967161.png)

By-T5-small tokenizer:

![grafik](https://user-images.githubusercontent.com/229382/126947200-44aefc4e-07f5-4051-86b4-cc89b785e5e2.png)


See colab code here: https://colab.research.google.com/drive/1nVxCerQon3hVA1RylZz7Be4N8LfjPgth?usp=sharing

Comments:
Pinging @patrickvonplaten 

See his (@patrickvonplaten  ) answer here: https://github.com/huggingface/transformers/pull/11971#issuecomment-889797262

pinging @Narsil

Hi, don't think rust is necessary here.

Using raw bytes should be just as fast in python (if not faster because no overhead).
Might require some heavy change though, most notably to remove the over reliance on regexp which is notably bad, especially with 100 of them in a single regexp.

@PhilipMay  if you want to tackle it, just modify this function: https://github.com/huggingface/transformers/blob/master/src/transformers/models/byt5/tokenization_byt5.py#L197 and remove all traces of regexp (or make sure it only runs once, is precompiled and an efficient one).

Do you mind giving a script to assess current speed and make sure modifications are speeding up too ? (Might have sometime at some point to tackle this).

Hi @Narsil ,
thanks for the answer.

I made some debugging:

![image](https://user-images.githubusercontent.com/229382/128864351-41586d85-d783-4f9b-84a2-31b080228bc7.png)

It seems like here:

https://github.com/huggingface/transformers/blob/2e0d767ab2bf8265a9f9b93adb1bc2084bc02849/src/transformers/tokenization_utils.py#L335-L350

It already splits `<pad>|
|<unk>` - see screenshot.

So for me it seems like we do not need this code:

https://github.com/huggingface/transformers/blob/2e0d767ab2bf8265a9f9b93adb1bc2084bc02849/src/transformers/models/byt5/tokenization_byt5.py#L207-L208

What do you think?


Changing the code to this:

```python
        # split on special characters
        # pattern = f"({'|'.join(self.special_tokens_encoder.keys())})"
        # sub_texts = list(filter(None, re.split(pattern, text)))
        sub_texts = text
```

Converts this: `"This <unk> is 
 some 
 text. <pad> other text!"` to this: 
`['T', 'h', 'i', 's', ' ', '<unk>', ' ', 'i', 's', ' ', '<', 's', '>', ' ', 's', 'o', 'm', 'e', ' ', '
', ' ', 't', 'e', 'x', 't', '.', ' ', '<pad>', ' ', 'o', 't', 'h', 'e', 'r', ' ', 't', 'e', 'x', 't', '!']`

Which seems to be ok...

Not sure if `
` is allowed to be split or not.
special_tokens contains something like 100 or so special tokens which most likely should be taken care of.

Can you run the tests ?

```
pytest -sv tests/test_tokenization_byt5.py
```

I expect your version is slightly incorrect, but I could be wrong.

instead of using `re.split(pattern, text)` if you manage to `self.regexp = re.compile(pattern)` (within __init__) and replace it with
`self.regexp.split(text)` that's already probably a speedup (with no change in functionality).

Edit: To be perfectly correct you need to recalculcate `self.regexp` everytime there's a change in special_tokens (`self.add_special_tokens` at least for instance) which would involved declaring a submethod to call the redefinition of `self.regexp`.


Letting @patrickvonplaten chime in if possible on correctness/speed for this.

Yeah `
` should not be split to single characters. It would also be important to make sure that newly added tokens of whatever character length are not split.

I think if all ByT5Tokenizer tests pass then a change to speed up the tokenizer is ok

> Yeah `
` should not be split to single characters.

I made a simple test:

```python
from transformers import ByT5Tokenizer
tok = ByT5Tokenizer.from_pretrained(pretrained_model_name_or_path="google/byt5-small")
token = tok.tokenize("This <unk> is <pad> a 
 test 
 with some special tokens.")
print(token)
```

It prints:

`['T', 'h', 'i', 's', ' ', '<unk>', ' ', 'i', 's', ' ', '<pad>', ' ', 'a', ' ', '
', ' ', 't', 'e', 's', 't', ' ', '<', 's', '>', ' ', 'w', 'i', 't', 'h', ' ', 's', 'o', 'm', 'e', ' ', 's', 'p', 'e', 'c', 'i', 'a', 'l', ' ', 't', 'o', 'k', 'e', 'n', 's', '.']`

So `
` is split. It does not split this: `<unk>, <pad> and 
`.

@Narsil and @patrickvonplaten in debugger it looks like this:

![image](https://user-images.githubusercontent.com/229382/129317805-453d7ad6-eb9e-4abc-b1c9-a2c3bf850262.png)

The pattern is `'(<pad>|
|<unk>)'` but NOT `
` or something else.

@PhilipMay Please provide a script for the benchmark, it would really help assess speed.

As for the example you're right `
` doesn't seem to be tokenized. (it's showcased in patrick's example)

> @PhilipMay Please provide a script for the benchmark, it would really help assess speed.

Like so?

```python

from transformers import ByT5Tokenizer
from datasets import load_dataset
import time
dataset = load_dataset('cnn_dailymail', '3.0.0', split='train')
articles = [d["article"] for d in dataset][:1000]
tok = ByT5Tokenizer.from_pretrained(pretrained_model_name_or_path="google/byt5-small")
start_time = time.time()
for a in articles:
    _ = tok.tokenize(a)
print("--- %s seconds ---" % (time.time() - start_time))
```

Mind checking out this: https://github.com/huggingface/transformers/pull/13119

I got something like 2X. Still suboptimal, but much of the overhead now lies in all the wrapping code, which would be much more tedious to remove. If you want to try, please go ahead ! 

Ok, the PR is already a nice boost. Turns out most of the performance loss is caused by special tokens (the 125 extra_ids), which are quite unlikely to appear in your text.

The current code (for slow tokenizers) is quite optimized for low number of special_tokens, which is not the case here.
If you are able to afford being incorrect (because you know your text doesn't contain <extra_id_XX> that should be processed) then, you can simply save the tokenizer, REMOVE those extra_ids and load it again.

Processing 1000 sentences
Current master:  2.4s
Optimize_byt5 branch: 0.47s
Without extra_ids : 0.07s

Is that enough for your use case ?

How to remove extra_ids simply:
```python
tok = ByT5Tokenizer.from_pretrained(pretrained_model_name_or_path="google/byt5-small")
# CAVEAT: This will break some functionality, use with caution
tok.unique_no_split_tokens = ["
", "<pad>", "<unk>"]
```