Avaiga / taipy-core

A Python library to build powerful and customized data-driven back-end applications.
Apache License 2.0
38 stars 17 forks source link

Fix/#697 - Performance issue when creating a new scenario #702

Closed trgiangdo closed 1 year ago

trgiangdo commented 1 year ago

https://github.com/Avaiga/taipy-core/issues/697

The issue is that when creating the first scenario on FS repository, several necessary files are not created yet, which makes the _load() method raises the ModelNotFound exception.

And since the default read_entity_retry value is 3, it would take about 2-3 seconds to finish creating the scenario, based on how many exception raised.

However, when creating the second scenario, it will be just fine.

Test code:

from taipy import Scope, Config, Frequency
from src.taipy.core import taipy as tp
from src.taipy.core import Core
import time

def replace(input_msg, template, name):
    return input_msg.replace(template, name)

input_msg_cfg = Config.configure_data_node(id="input_msg", scope=Scope.GLOBAL, default_data="Hello, today it's <TPL>!")
template_cfg = Config.configure_data_node(id="template", scope=Scope.GLOBAL, default_data="<TPL>")
date_cfg = Config.configure_data_node(id="date")
message_cfg = Config.configure_data_node(id="msg")

replace_template_cfg = Config.configure_task("replace", replace, [input_msg_cfg, template_cfg, date_cfg], message_cfg)
scenario_daily_cfg = Config.configure_scenario_from_tasks(
    id="scenario_daily_cfg", task_configs=[replace_template_cfg], frequency=Frequency.DAILY
)
scenario_cfg = Config.configure_scenario_from_tasks(id="scenario_cfg", task_configs=[replace_template_cfg])

Core().run()

start = time.time()
scenario = tp.create_scenario(scenario_cfg)
print(f"Create scenario time: {time.time() - start}")
start = time.time()
tp.submit(scenario)
print(f"Submit scenario time: {time.time() - start}")

start = time.time()
scenario_2 = tp.create_scenario(scenario_cfg)
print(f"Create scenario 2 time: {time.time() - start}")
start = time.time()
tp.submit(scenario_2)
print(f"Submit scenario 2 time: {time.time() - start}")

Result:

[2023-07-25 11:36:35][Taipy][INFO] Development mode: Clean all entities of version a4b4a09f-ecad-465b-bbdf-ceda3077f2f4
[2023-07-25 11:36:38][Taipy][ERROR] DataNode not found: DATANODE_input_msg_c9ca66fe-5a3e-46cc-a7ef-379c5dff64e5
[2023-07-25 11:36:40][Taipy][ERROR] DataNode not found: DATANODE_template_a544c333-bf3c-42a3-b8ea-06414ebb5183
Create scenario time: 3.0631818771362305
[2023-07-25 11:36:40][Taipy][WARNING] DATANODE_date_a6bf9c3f-697a-402a-a825-d00245cfe784 cannot be read because it has never been written. Hint: The data node may refer to a wrong path : .data/pickles/DATANODE_date_a6bf9c3f-697a-402a-a825-d00245cfe784.p 
Submit scenario time: 0.0188140869140625
Create scenario 2 time: 0.03878903388977051
[2023-07-25 11:36:40][Taipy][WARNING] DATANODE_date_3fee7bdd-300f-4edd-94dd-4564bd339ed3 cannot be read because it has never been written. Hint: The data node may refer to a wrong path : .data/pickles/DATANODE_date_3fee7bdd-300f-4edd-94dd-4564bd339ed3.p 
Submit scenario 2 time: 0.014927148818969727

You can see that in the result, the time to create scenario_2 is 0.03 second, which is just fine.

There are 2 possible solutions that I can think of.

  1. Instead of retry on all Exception, we only retry on json.JSONDecodeError, which was raised by Fred a while ago. (The exception that is causing the performance issue is ModelNotFound, which is catch at the _Manager._get() method). This is also what I implemented.
  2. Reduce the default read_entity_retry to 1, and maybe reduce the sleep time (0.5s maybe too long, idk). This will let the creation performance of the first scenario be worse than the next. It won't be too long anyway.
github-actions[bot] commented 1 year ago

☂️ Python Cov

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
7370 7081 96% 85% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/taipy/core/_repository/_filesystem_repository.py 97% 🟢
src/taipy/core/common/_utils.py 100% 🟢
src/taipy/core/common/typing.py 100% 🟢
src/taipy/core/config/core_section.py 97% 🟢
src/taipy/core/exceptions/exceptions.py 91% 🟢
TOTAL 97% 🟢

updated for commit: 966f002 by action🐍

trgiangdo commented 1 year ago

0.5s is also kinda long IMO, for decent computer storage should be pretty fast, 0.2s seem more reasonable for me for almost "instant" feedback.

Agree. A 0.1 or 0.2 seconds sleep should be enough

jrobinAV commented 1 year ago

There is something I don't understand.

We changed the default Core config to set the retry nb to 3. Right? Before the change, it was not by default in Core but it was set to 3 in Taipy Core viz elements library.

So, in the end, I don't understand why the behavior is different. It should be the exact same behavior from the Core viz elements standpoint.

trgiangdo commented 1 year ago

As I understand, the read_entity_retry was set to 3 in src/taipy/gui_core/GuiCoreLib.py in taipy.

In src/taipy/__init__.py, Core is initialized before GuiCoreLib, so probably setting the read_entity_retry did not take effect at all (because the value of Config.global_config.read_entity_retry is already passed to the wrapper). Which is really weird

trgiangdo commented 1 year ago

@toan-quach @jrobinAV I implemented Toan's idea, however, there was an issue.

The _load() method always need to raise the ModelNotFound exception when there is an error (for the _Manager_get() method). If we modify the catch on manager level, it can cause hidden bug (since we have multiple repository types).

Instead, I notice that we only want to retry_read_entity when reading a file. So I split the responsibility to a separate private method, and only retry at that level.

There is 1 more question remains: Do we only retry when the file can not be read, or do we also retry when the file doesn't exist?

Please let me know what you think.

jrobinAV commented 1 year ago

There is 1 more question remains: Do we only retry when the file can not be read, or do we also retry when the file doesn't exist?

I believe only when the file exists and cannot be read. There is no point retrying when the file does not exist yet.

Thx,