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

Add --preferred-encoding (gzip|brotli) to use when tile is not pre-encoded by source #1189

Merged
merged 22 commits into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/src/config-file.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ worker_processes: 8
# Amount of memory (in MB) to use for caching tiles [default: 512, 0 to disable]
cache_size_mb: 1024

# Preferred tiles encoding, gzip or brotli, default brotili. You could also use br as a shortcut for brotli
nyurik marked this conversation as resolved.
Show resolved Hide resolved
preferred_encoding: gzip

# Database configuration. This can also be a list of PG configs.
postgres:
# Database connection string. You can use env vars too, for example:
Expand Down
5 changes: 5 additions & 0 deletions docs/src/run-with-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ Options:
-W, --workers <WORKERS>
Number of web server workers

--preferred-encoding <PREFERRED_ENCODING>
nyurik marked this conversation as resolved.
Show resolved Hide resolved
Preferred tiles encoding. gzip or brotli, default brotili. You could also use br as a shortcut for brotli

[possible values: brotli, gzip]

-b, --auto-bounds <AUTO_BOUNDS>
Specify how bounds should be computed for the spatial PG tables. [DEFAULT: quick]

Expand Down
2 changes: 1 addition & 1 deletion martin/benches/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl Source for NullSource {
}

async fn process_tile(sources: &TileSources) {
let src = DynTileSource::new(sources, "null", Some(0), "", None, None).unwrap();
let src = DynTileSource::new(sources, "null", Some(0), "", None, None, None).unwrap();
src.get_http_response(TileCoord { z: 0, x: 0, y: 0 })
.await
.unwrap();
Expand Down
1 change: 1 addition & 0 deletions martin/src/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ mod root;
pub use root::{Args, ExtraArgs, MetaArgs};

mod srv;
pub use srv::PreferredEncoding;
pub use srv::SrvArgs;
24 changes: 24 additions & 0 deletions martin/src/args/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ pub fn parse_file_args<T: crate::file_config::ConfigExtras>(

#[cfg(test)]
mod tests {

use super::*;
use crate::args::PreferredEncoding;
use crate::test_utils::FauxEnv;
use crate::MartinError::UnrecognizableConnections;

Expand Down Expand Up @@ -215,6 +217,28 @@ mod tests {
assert_eq!(args, (cfg, meta));
}

#[test]
fn cli_encoding_arguments() {
let config1 = parse(&["martin", "--preferred-encoding", "brotli"]);
let config2 = parse(&["martin", "--preferred-encoding", "br"]);
let config3 = parse(&["martin", "--preferred-encoding", "gzip"]);
let config4 = parse(&["martin"]);

assert_eq!(
config1.unwrap().0.srv.preferred_encoding,
Some(PreferredEncoding::Brotli)
);
assert_eq!(
config2.unwrap().0.srv.preferred_encoding,
Some(PreferredEncoding::Brotli)
);
assert_eq!(
config3.unwrap().0.srv.preferred_encoding,
Some(PreferredEncoding::Gzip)
);
assert_eq!(config4.unwrap().0.srv.preferred_encoding, None);
}

#[test]
fn cli_bad_arguments() {
for params in [
Expand Down
18 changes: 18 additions & 0 deletions martin/src/args/srv.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::srv::{SrvConfig, KEEP_ALIVE_DEFAULT, LISTEN_ADDRESSES_DEFAULT};
use clap::ValueEnum;
use serde::{Deserialize, Serialize};

#[derive(clap::Args, Debug, PartialEq, Default)]
#[command(about, version)]
Expand All @@ -10,6 +12,19 @@ pub struct SrvArgs {
/// Number of web server workers
#[arg(short = 'W', long)]
pub workers: Option<usize>,
/// Preferred tiles encoding. gzip or brotli, default brotili. You could also use br as a shortcut for brotli
nyurik marked this conversation as resolved.
Show resolved Hide resolved
#[arg(long)]
pub preferred_encoding: Option<PreferredEncoding>,
}

#[derive(PartialEq, Eq, Default, Debug, Clone, Copy, Serialize, Deserialize, ValueEnum)]
#[serde(rename_all = "lowercase")]
pub enum PreferredEncoding {
#[default]
#[serde(alias = "br")]
#[clap(alias("br"))]
Brotli,
Gzip,
}

impl SrvArgs {
Expand All @@ -24,5 +39,8 @@ impl SrvArgs {
if self.workers.is_some() {
srv_config.worker_processes = self.workers;
}
if self.preferred_encoding.is_some() {
srv_config.preferred_encoding = self.preferred_encoding;
}
}
}
1 change: 1 addition & 0 deletions martin/src/bin/martin-cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ async fn run_tile_copy(args: CopyArgs, state: ServerState) -> MartinCpResult<()>
args.url_query.as_deref().unwrap_or_default(),
Some(parse_encoding(args.encoding.as_str())?),
None,
None,
)?;
// parallel async below uses move, so we must only use copyable types
let src = &src;
Expand Down
36 changes: 35 additions & 1 deletion martin/src/srv/config.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use serde::{Deserialize, Serialize};

use crate::args::PreferredEncoding;

pub const KEEP_ALIVE_DEFAULT: u64 = 75;
pub const LISTEN_ADDRESSES_DEFAULT: &str = "0.0.0.0:3000";

Expand All @@ -9,6 +11,7 @@ pub struct SrvConfig {
pub keep_alive: Option<u64>,
pub listen_addresses: Option<String>,
pub worker_processes: Option<usize>,
pub preferred_encoding: Option<PreferredEncoding>,
}

#[cfg(test)]
Expand All @@ -19,18 +22,49 @@ mod tests {
use crate::test_utils::some;

#[test]
fn parse_empty_config() {
fn parse_config() {
assert_eq!(
serde_yaml::from_str::<SrvConfig>(indoc! {"
keep_alive: 75
listen_addresses: '0.0.0.0:3000'
worker_processes: 8
"})
.unwrap(),
SrvConfig {
keep_alive: Some(75),
listen_addresses: some("0.0.0.0:3000"),
worker_processes: Some(8),
preferred_encoding: None,
}
);
assert_eq!(
serde_yaml::from_str::<SrvConfig>(indoc! {"
keep_alive: 75
listen_addresses: '0.0.0.0:3000'
worker_processes: 8
preferred_encoding: br
"})
.unwrap(),
SrvConfig {
keep_alive: Some(75),
listen_addresses: some("0.0.0.0:3000"),
worker_processes: Some(8),
preferred_encoding: Some(PreferredEncoding::Brotli),
}
);
assert_eq!(
serde_yaml::from_str::<SrvConfig>(indoc! {"
keep_alive: 75
listen_addresses: '0.0.0.0:3000'
worker_processes: 8
preferred_encoding: brotli
"})
.unwrap(),
SrvConfig {
keep_alive: Some(75),
listen_addresses: some("0.0.0.0:3000"),
worker_processes: Some(8),
preferred_encoding: Some(PreferredEncoding::Brotli),
}
);
}
Expand Down
14 changes: 8 additions & 6 deletions martin/src/srv/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ type Server = Pin<Box<dyn Future<Output = MartinResult<()>>>>;
pub fn new_server(config: SrvConfig, state: ServerState) -> MartinResult<(Server, String)> {
let catalog = Catalog::new(&state)?;

let keep_alive = Duration::from_secs(config.keep_alive.unwrap_or(KEEP_ALIVE_DEFAULT));
let worker_processes = config.worker_processes.unwrap_or_else(num_cpus::get);
let listen_addresses = config
.listen_addresses
.clone()
.unwrap_or_else(|| LISTEN_ADDRESSES_DEFAULT.to_owned());

let factory = move || {
let cors_middleware = Cors::default()
.allow_any_origin()
Expand All @@ -124,6 +131,7 @@ pub fn new_server(config: SrvConfig, state: ServerState) -> MartinResult<(Server
let app = app.app_data(Data::new(state.fonts.clone()));

app.app_data(Data::new(catalog.clone()))
.app_data(Data::new(config.clone()))
.wrap(cors_middleware)
.wrap(middleware::NormalizePath::new(TrailingSlash::MergeOnly))
.wrap(middleware::Logger::default())
Expand All @@ -136,12 +144,6 @@ pub fn new_server(config: SrvConfig, state: ServerState) -> MartinResult<(Server
return Ok((Box::pin(server), "(aws lambda)".into()));
}

let keep_alive = Duration::from_secs(config.keep_alive.unwrap_or(KEEP_ALIVE_DEFAULT));
let worker_processes = config.worker_processes.unwrap_or_else(num_cpus::get);
let listen_addresses = config
.listen_addresses
.unwrap_or_else(|| LISTEN_ADDRESSES_DEFAULT.to_owned());

let server = HttpServer::new(factory)
.bind(listen_addresses.clone())
.map_err(|e| BindingError(e, listen_addresses.clone()))?
Expand Down
93 changes: 81 additions & 12 deletions martin/src/srv/tiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,17 @@ use log::trace;
use martin_tile_utils::{Encoding, Format, TileInfo};
use serde::Deserialize;

use crate::args::PreferredEncoding;
use crate::source::{Source, TileSources, UrlQuery};
use crate::srv::server::map_internal_error;
use crate::srv::SrvConfig;
use crate::utils::cache::get_or_insert_cached_value;
use crate::utils::{
decode_brotli, decode_gzip, encode_brotli, encode_gzip, CacheKey, CacheValue, MainCache,
OptMainCache,
};
use crate::{Tile, TileCoord, TileData};

static SUPPORTED_ENCODINGS: &[HeaderEnc] = &[
HeaderEnc::brotli(),
HeaderEnc::gzip(),
HeaderEnc::identity(),
];

#[derive(Deserialize, Clone)]
pub struct TileRequest {
source_ids: String,
Expand All @@ -36,6 +32,7 @@ pub struct TileRequest {
#[route("/{source_ids}/{z}/{x}/{y}", method = "GET", method = "HEAD")]
async fn get_tile(
req: HttpRequest,
srv_config: Data<SrvConfig>,
path: Path<TileRequest>,
sources: Data<TileSources>,
cache: Data<OptMainCache>,
Expand All @@ -46,6 +43,7 @@ async fn get_tile(
Some(path.z),
req.query_string(),
req.get_header::<AcceptEncoding>(),
srv_config.preferred_encoding,
cache.as_ref().as_ref(),
)?;

Expand All @@ -62,7 +60,8 @@ pub struct DynTileSource<'a> {
pub info: TileInfo,
pub query_str: Option<&'a str>,
pub query_obj: Option<UrlQuery>,
pub encodings: Option<AcceptEncoding>,
pub accept_encodings: Option<AcceptEncoding>,
pub prefered_encodings: Option<PreferredEncoding>,
pub cache: Option<&'a MainCache>,
}

Expand All @@ -72,7 +71,8 @@ impl<'a> DynTileSource<'a> {
source_ids: &str,
zoom: Option<u8>,
query: &'a str,
encodings: Option<AcceptEncoding>,
accpect_encodings: Option<AcceptEncoding>,
preferred_encoding: Option<PreferredEncoding>,
cache: Option<&'a MainCache>,
) -> ActixResult<Self> {
let (sources, use_url_query, info) = sources.get_sources(source_ids, zoom)?;
Expand All @@ -93,7 +93,8 @@ impl<'a> DynTileSource<'a> {
info,
query_str,
query_obj,
encodings,
accept_encodings: accpect_encodings,
nyurik marked this conversation as resolved.
Show resolved Hide resolved
prefered_encodings: preferred_encoding,
cache,
})
}
Expand Down Expand Up @@ -169,7 +170,7 @@ impl<'a> DynTileSource<'a> {

fn recompress(&self, tile: TileData) -> ActixResult<Tile> {
let mut tile = Tile::new(tile, self.info);
if let Some(accept_enc) = &self.encodings {
if let Some(accept_enc) = &self.accept_encodings {
if self.info.encoding.is_encoded() {
// already compressed, see if we can send it as is, or need to re-compress
if !accept_enc.iter().any(|e| {
Expand All @@ -184,9 +185,30 @@ impl<'a> DynTileSource<'a> {
}
}
if tile.info.encoding == Encoding::Uncompressed {
let ordered_encodings = if let Some(prefered) = self.prefered_encodings {
match prefered {
PreferredEncoding::Brotli => [
nyurik marked this conversation as resolved.
Show resolved Hide resolved
HeaderEnc::brotli(),
HeaderEnc::gzip(),
HeaderEnc::identity(),
],
PreferredEncoding::Gzip => [
HeaderEnc::gzip(),
HeaderEnc::brotli(),
HeaderEnc::identity(),
],
}
} else {
[
HeaderEnc::brotli(),
HeaderEnc::gzip(),
HeaderEnc::identity(),
]
};
// only apply compression if the content supports it
if let Some(HeaderEnc::Known(enc)) =
accept_enc.negotiate(SUPPORTED_ENCODINGS.iter())
// accept_enc.negotiate(SUPPORTED_ENCODINGS.iter())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// accept_enc.negotiate(SUPPORTED_ENCODINGS.iter())

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Deleted.

Copy link
Collaborator Author

@sharkAndshark sharkAndshark Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Preferred tile encoding when martin is negotiating with browser, only works if the tile itself is not pre-compressed. The default Brotli is much smaller(and may be faster with caching) but a little bit slower than gzip."
Well this version maybe much worse..

accept_enc.negotiate(ordered_encodings.iter())
{
// (re-)compress the tile into the preferred encoding
tile = encode(tile, enc)?;
Expand Down Expand Up @@ -251,6 +273,53 @@ mod tests {
use super::*;
use crate::srv::server::tests::TestSource;

#[actix_rt::test]
async fn test_encoding_preference() {
let source = TestSource {
id: "test_source",
tj: tilejson! { tiles: vec![] },
data: vec![1_u8, 2, 3],
};
let sources = TileSources::new(vec![vec![Box::new(source)]]);

for (accept_encodings, prefered_encoding, result_encoding) in [
(
Some(AcceptEncoding(vec![
"gzip;q=1".parse().unwrap(),
"br;q=1".parse().unwrap(),
])),
Some(PreferredEncoding::Brotli),
Encoding::Brotli,
),
(
Some(AcceptEncoding(vec![
"gzip;q=1".parse().unwrap(),
"br;q=0.5".parse().unwrap(),
])),
Some(PreferredEncoding::Brotli),
Encoding::Gzip,
),
] {
let src = DynTileSource::new(
&sources,
"test_source",
None,
"",
accept_encodings,
prefered_encoding,
None,
)
.unwrap();
let xyz = TileCoord { z: 0, x: 0, y: 0 };
let data = &src.get_tile_content(xyz).await.unwrap().data;
let decoded = match result_encoding {
Encoding::Gzip => decode_gzip(data),
Encoding::Brotli => decode_brotli(data),
_ => panic!("Unexpected encoding"),
};
assert_eq!(vec![1_u8, 2, 3], decoded.unwrap());
}
}
#[actix_rt::test]
async fn test_tile_content() {
let non_empty_source = TestSource {
Expand Down Expand Up @@ -278,7 +347,7 @@ mod tests {
("empty,non-empty", vec![1_u8, 2, 3]),
("empty,non-empty,empty", vec![1_u8, 2, 3]),
] {
let src = DynTileSource::new(&sources, source_id, None, "", None, None).unwrap();
let src = DynTileSource::new(&sources, source_id, None, "", None, None, None).unwrap();
let xyz = TileCoord { z: 0, x: 0, y: 0 };
assert_eq!(expected, &src.get_tile_content(xyz).await.unwrap().data);
}
Expand Down
Loading
Loading